From d8dec61ae991761246741fd286701b243fca5712 Mon Sep 17 00:00:00 2001 From: Istio Automation Date: Mon, 26 Aug 2024 22:35:44 -0400 Subject: [PATCH 1/7] Automator: update proxy@release-1.22 in istio/istio@release-1.22 (#52837) --- istio.deps | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/istio.deps b/istio.deps index 09115cce8e96..01213541ccff 100644 --- a/istio.deps +++ b/istio.deps @@ -4,7 +4,7 @@ "name": "PROXY_REPO_SHA", "repoName": "proxy", "file": "", - "lastStableSHA": "9603bcb6fd358cb0affbb662630751d77ab66efb" + "lastStableSHA": "41f0598c8d4514c4c7e17b5f56a064efdce0bf9d" }, { "_comment": "", From 05468d6be3e0f98088528cadddbf6fb62ac3fd49 Mon Sep 17 00:00:00 2001 From: John Howard Date: Fri, 30 Aug 2024 18:33:22 -0700 Subject: [PATCH 2/7] 1.22: Revert "[release-1.22] networking: match multiple VIPs in sidecar outbound listener (#52952) * Revert "[release-1.22] networking: match multiple VIPs in sidecar outbound listener (#52283)" This reverts commit 16b6f60de5c82995095eab87afc849d301edcd91. * Add release note --- pilot/pkg/model/service.go | 62 +------- pilot/pkg/model/service_test.go | 144 ------------------ pilot/pkg/networking/core/listener.go | 11 +- pilot/pkg/networking/core/tls.go | 24 ++- .../serviceentry/controller.go | 2 +- .../serviceentry/controller_test.go | 13 +- .../serviceentry/conversion.go | 66 ++++---- .../serviceentry/conversion_test.go | 50 +++--- .../serviceentry/store_test.go | 5 +- pilot/pkg/xds/eds_test.go | 7 +- releasenotes/notes/51967-revert.yaml | 9 ++ releasenotes/notes/51967.yaml | 8 - tests/integration/pilot/common/routing.go | 124 --------------- tests/integration/pilot/common/traffic.go | 1 - .../pilot/dns_auto_allocation_test.go | 104 ------------- 15 files changed, 108 insertions(+), 522 deletions(-) create mode 100644 releasenotes/notes/51967-revert.yaml delete mode 100644 releasenotes/notes/51967.yaml delete mode 100644 tests/integration/pilot/dns_auto_allocation_test.go diff --git a/pilot/pkg/model/service.go b/pilot/pkg/model/service.go index 3adf30a37095..ca75b08e8068 100644 --- a/pilot/pkg/model/service.go +++ b/pilot/pkg/model/service.go @@ -1270,33 +1270,14 @@ func (s *Service) GetAddressForProxy(node *Proxy) string { // GetExtraAddressesForProxy returns a k8s service's extra addresses to the cluster where the node resides. // Especially for dual stack k8s service to get other IP family addresses. func (s *Service) GetExtraAddressesForProxy(node *Proxy) []string { - addresses := s.getAllAddressesForProxy(node) - if len(addresses) > 1 { - return addresses[1:] - } - return nil -} - -// GetAllAddressesForProxy returns a k8s service's extra addresses to the cluster where the node resides. -// Especially for dual stack k8s service to get other IP family addresses. -func (s *Service) GetAllAddressesForProxy(node *Proxy) []string { - return s.getAllAddressesForProxy(node) -} - -func (s *Service) getAllAddressesForProxy(node *Proxy) []string { - if node.Metadata != nil && node.Metadata.ClusterID != "" { - addresses := s.ClusterVIPs.GetAddressesFor(node.Metadata.ClusterID) - if (features.EnableDualStack || features.EnableAmbient) && len(addresses) > 0 { - return addresses - } - addresses = filterAddresses(addresses, node.SupportsIPv4(), node.SupportsIPv6()) - if len(addresses) > 0 { - return addresses + if features.EnableDualStack && node.Metadata != nil { + if node.Metadata.ClusterID != "" { + addresses := s.ClusterVIPs.GetAddressesFor(node.Metadata.ClusterID) + if len(addresses) > 1 { + return addresses[1:] + } } } - if a := s.GetAddressForProxy(node); a != "" { - return []string{a} - } return nil } @@ -1311,37 +1292,6 @@ func (s *Service) getAllAddresses() []string { return addresses } -func filterAddresses(addresses []string, supportsV4, supportsV6 bool) []string { - var ipv4Addresses []string - var ipv6Addresses []string - for _, addr := range addresses { - // check if an address is a CIDR range - if strings.Contains(addr, "/") { - if prefix, err := netip.ParsePrefix(addr); err != nil { - log.Warnf("failed to parse prefix address '%s': %s", addr, err) - continue - } else if supportsV4 && prefix.Addr().Is4() { - ipv4Addresses = append(ipv4Addresses, addr) - } else if supportsV6 && prefix.Addr().Is6() { - ipv6Addresses = append(ipv6Addresses, addr) - } - } else { - if ipAddr, err := netip.ParseAddr(addr); err != nil { - log.Warnf("failed to parse address '%s': %s", addr, err) - continue - } else if supportsV4 && ipAddr.Is4() { - ipv4Addresses = append(ipv4Addresses, addr) - } else if supportsV6 && ipAddr.Is6() { - ipv6Addresses = append(ipv6Addresses, addr) - } - } - } - if len(ipv4Addresses) > 0 { - return ipv4Addresses - } - return ipv6Addresses -} - // GetTLSModeFromEndpointLabels returns the value of the label // security.istio.io/tlsMode if set. Do not return Enums or constants // from this function as users could provide values other than istio/disabled diff --git a/pilot/pkg/model/service_test.go b/pilot/pkg/model/service_test.go index 31e3f38e6708..e174d9f85e67 100644 --- a/pilot/pkg/model/service_test.go +++ b/pilot/pkg/model/service_test.go @@ -21,13 +21,11 @@ import ( "github.com/google/go-cmp/cmp/cmpopts" fuzz "github.com/google/gofuzz" - "istio.io/istio/pilot/pkg/features" "istio.io/istio/pkg/cluster" "istio.io/istio/pkg/config/constants" "istio.io/istio/pkg/config/host" "istio.io/istio/pkg/config/labels" "istio.io/istio/pkg/config/visibility" - "istio.io/istio/pkg/test" "istio.io/istio/pkg/test/util/assert" ) @@ -588,145 +586,3 @@ func TestParseSubsetKeyHostname(t *testing.T) { }) } } - -func TestGetAllAddresses(t *testing.T) { - tests := []struct { - name string - service *Service - ipMode IPMode - dualStackEnabled bool - ambientEnabled bool - autoAllocationEnabled bool - expectedAddresses []string - expectedExtraAddresses []string - }{ - { - name: "IPv4 mode, IPv4 and IPv6 CIDR addresses, expected to return only IPv4 addresses", - service: &Service{ - DefaultAddress: "10.0.0.0/28", - ClusterVIPs: AddressMap{ - Addresses: map[cluster.ID][]string{ - "id": {"10.0.0.0/28", "10.0.0.16/28", "::ffff:10.0.0.32/96", "::ffff:10.0.0.48/96"}, - }, - }, - }, - ipMode: IPv4, - expectedAddresses: []string{"10.0.0.0/28", "10.0.0.16/28"}, - expectedExtraAddresses: []string{"10.0.0.16/28"}, - }, - { - name: "IPv6 mode, IPv4 and IPv6 CIDR addresses, expected to return only IPv6 addresses", - service: &Service{ - DefaultAddress: "10.0.0.0/28", - ClusterVIPs: AddressMap{ - Addresses: map[cluster.ID][]string{ - "id": {"10.0.0.0/28", "10.0.0.16/28", "::ffff:10.0.0.32/96", "::ffff:10.0.0.48/96"}, - }, - }, - }, - ipMode: IPv6, - expectedAddresses: []string{"::ffff:10.0.0.32/96", "::ffff:10.0.0.48/96"}, - expectedExtraAddresses: []string{"::ffff:10.0.0.48/96"}, - }, - { - name: "dual mode, ISTIO_DUAL_STACK disabled, IPv4 and IPv6 addresses, expected to return only IPv4 addresses", - service: &Service{ - DefaultAddress: "10.0.0.0", - ClusterVIPs: AddressMap{ - Addresses: map[cluster.ID][]string{ - "id": {"10.0.0.0", "10.0.0.16", "::ffff:10.0.0.32", "::ffff:10.0.0.48"}, - }, - }, - }, - ipMode: Dual, - expectedAddresses: []string{"10.0.0.0", "10.0.0.16"}, - expectedExtraAddresses: []string{"10.0.0.16"}, - }, - { - name: "dual mode, ISTIO_DUAL_STACK enabled, IPv4 and IPv6 addresses, expected to return only IPv4 addresses", - service: &Service{ - DefaultAddress: "10.0.0.0", - ClusterVIPs: AddressMap{ - Addresses: map[cluster.ID][]string{ - "id": {"10.0.0.0", "10.0.0.16", "::ffff:10.0.0.32", "::ffff:10.0.0.48"}, - }, - }, - }, - ipMode: Dual, - dualStackEnabled: true, - expectedAddresses: []string{"10.0.0.0", "10.0.0.16", "::ffff:10.0.0.32", "::ffff:10.0.0.48"}, - expectedExtraAddresses: []string{"10.0.0.16", "::ffff:10.0.0.32", "::ffff:10.0.0.48"}, - }, - { - name: "IPv4 mode, ISTIO_DUAL_STACK disabled, ambient enabled, IPv4 and IPv6 addresses, expected to return all addresses", - service: &Service{ - DefaultAddress: "10.0.0.0/28", - ClusterVIPs: AddressMap{ - Addresses: map[cluster.ID][]string{ - "id": {"10.0.0.0/28", "10.0.0.16/28", "::ffff:10.0.0.32", "::ffff:10.0.0.48"}, - }, - }, - }, - ipMode: IPv4, - ambientEnabled: true, - expectedAddresses: []string{"10.0.0.0/28", "10.0.0.16/28", "::ffff:10.0.0.32", "::ffff:10.0.0.48"}, - expectedExtraAddresses: []string{"10.0.0.16/28", "::ffff:10.0.0.32", "::ffff:10.0.0.48"}, - }, - { - name: "IPv6 mode, ISTIO_DUAL_STACK disabled, ambient enabled, IPv4 and IPv6 addresses, expected to return all addresses", - service: &Service{ - DefaultAddress: "10.0.0.0/28", - ClusterVIPs: AddressMap{ - Addresses: map[cluster.ID][]string{ - "id": {"10.0.0.0/28", "10.0.0.16/28", "::ffff:10.0.0.32", "::ffff:10.0.0.48"}, - }, - }, - }, - ipMode: IPv6, - ambientEnabled: true, - expectedAddresses: []string{"10.0.0.0/28", "10.0.0.16/28", "::ffff:10.0.0.32", "::ffff:10.0.0.48"}, - expectedExtraAddresses: []string{"10.0.0.16/28", "::ffff:10.0.0.32", "::ffff:10.0.0.48"}, - }, - { - name: "IPv4 mode, auto-allocation enabled, expected auto-allocated address", - service: &Service{ - DefaultAddress: "0.0.0.0", - AutoAllocatedIPv4Address: "240.240.0.1", - }, - ipMode: IPv4, - autoAllocationEnabled: true, - expectedAddresses: []string{"240.240.0.1"}, - expectedExtraAddresses: []string{}, - }, - { - name: "IPv6 mode, auto-allocation enabled, expected auto-allocated address", - service: &Service{ - DefaultAddress: "0.0.0.0", - AutoAllocatedIPv6Address: "2001:2::f0f0:e351", - }, - ipMode: IPv6, - autoAllocationEnabled: true, - expectedAddresses: []string{"2001:2::f0f0:e351"}, - expectedExtraAddresses: []string{}, - }, - } - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - if tc.dualStackEnabled { - test.SetForTest(t, &features.EnableDualStack, true) - } - if tc.ambientEnabled { - test.SetForTest(t, &features.EnableAmbient, true) - } - proxy := &Proxy{Metadata: &NodeMetadata{ClusterID: "id"}, ipMode: tc.ipMode} - if tc.autoAllocationEnabled { - proxy.Metadata.DNSCapture = true - proxy.Metadata.DNSAutoAllocate = true - } - addresses := tc.service.GetAllAddressesForProxy(proxy) - assert.Equal(t, addresses, tc.expectedAddresses) - extraAddresses := tc.service.GetExtraAddressesForProxy(proxy) - assert.Equal(t, extraAddresses, tc.expectedExtraAddresses) - }) - } -} diff --git a/pilot/pkg/networking/core/listener.go b/pilot/pkg/networking/core/listener.go index f0fdc4307097..cdd71bdb6852 100644 --- a/pilot/pkg/networking/core/listener.go +++ b/pilot/pkg/networking/core/listener.go @@ -264,10 +264,10 @@ func (l listenerBinding) Primary() string { // Extra returns any additional bindings. This is always empty if dual stack is disabled func (l listenerBinding) Extra() []string { - if len(l.binds) > 1 { - return l.binds[1:] + if !features.EnableDualStack || len(l.binds) == 1 { + return nil } - return nil + return l.binds[1:] } type outboundListenerEntry struct { @@ -820,8 +820,9 @@ func (lb *ListenerBuilder) buildSidecarOutboundListener(listenerOpts outboundLis } else { // Address is a CIDR. Fall back to 0.0.0.0 and // filter chain match + // TODO: this probably needs to handle dual stack better listenerOpts.bind.binds = actualWildcards - listenerOpts.cidr = append([]string{svcListenAddress}, svcExtraListenAddresses...) + listenerOpts.cidr = svcListenAddress } } } @@ -1060,7 +1061,7 @@ type outboundListenerOpts struct { proxy *model.Proxy bind listenerBinding - cidr []string + cidr string port *model.Port service *model.Service diff --git a/pilot/pkg/networking/core/tls.go b/pilot/pkg/networking/core/tls.go index c1b44bcafaf1..501ded47bc38 100644 --- a/pilot/pkg/networking/core/tls.go +++ b/pilot/pkg/networking/core/tls.go @@ -100,7 +100,7 @@ func hashRuntimeTLSMatchPredicates(match *v1alpha3.TLSMatchAttributes) string { return strings.Join(match.SniHosts, ",") + "|" + strings.Join(match.DestinationSubnets, ",") } -func buildSidecarOutboundTLSFilterChainOpts(node *model.Proxy, push *model.PushContext, destinationCIDRs []string, +func buildSidecarOutboundTLSFilterChainOpts(node *model.Proxy, push *model.PushContext, destinationCIDR string, service *model.Service, bind string, listenPort *model.Port, gateways sets.String, configs []config.Config, ) []*filterChainOpts { @@ -142,7 +142,11 @@ func buildSidecarOutboundTLSFilterChainOpts(node *model.Proxy, push *model.PushC // Use the service's CIDRs. // But if a virtual service overrides it with its own destination subnet match // give preference to the user provided one - // destinationCIDRs will be empty for services with VIPs + // destinationCIDR will be empty for services with VIPs + var destinationCIDRs []string + if destinationCIDR != "" { + destinationCIDRs = []string{destinationCIDR} + } // Only set CIDR match if the listener is bound to an IP. // If its bound to a unix domain socket, then ignore the CIDR matches // Unix domain socket bound ports have Port value set to 0 @@ -205,7 +209,7 @@ func buildSidecarOutboundTLSFilterChainOpts(node *model.Proxy, push *model.PushC svcListenAddress = constants.UnspecifiedIPv6 } - if len(destinationCIDRs) > 0 || len(svcListenAddress) == 0 || (svcListenAddress == actualWildcard && bind == actualWildcard) { + if len(destinationCIDR) > 0 || len(svcListenAddress) == 0 || (svcListenAddress == actualWildcard && bind == actualWildcard) { sniHosts = []string{string(service.Hostname)} for _, a := range service.Attributes.Aliases { alt := GenerateAltVirtualHosts(a.Hostname.String(), 0, node.DNSDomain) @@ -215,6 +219,10 @@ func buildSidecarOutboundTLSFilterChainOpts(node *model.Proxy, push *model.PushC } destinationRule := CastDestinationRule(node.SidecarScope.DestinationRule( model.TrafficDirectionOutbound, node, service.Hostname).GetRule()) + var destinationCIDRs []string + if destinationCIDR != "" { + destinationCIDRs = []string{destinationCIDR} + } out = append(out, &filterChainOpts{ sniHosts: sniHosts, destinationCIDRs: destinationCIDRs, @@ -226,7 +234,7 @@ func buildSidecarOutboundTLSFilterChainOpts(node *model.Proxy, push *model.PushC return out } -func buildSidecarOutboundTCPFilterChainOpts(node *model.Proxy, push *model.PushContext, destinationCIDRs []string, +func buildSidecarOutboundTCPFilterChainOpts(node *model.Proxy, push *model.PushContext, destinationCIDR string, service *model.Service, listenPort *model.Port, gateways sets.String, configs []config.Config, ) []*filterChainOpts { @@ -245,6 +253,10 @@ TcpLoop: for _, cfg := range configs { virtualService := cfg.Spec.(*v1alpha3.VirtualService) for _, tcp := range virtualService.Tcp { + var destinationCIDRs []string + if destinationCIDR != "" { + destinationCIDRs = []string{destinationCIDR} + } if len(tcp.Match) == 0 { // implicit match out = append(out, &filterChainOpts{ @@ -321,6 +333,10 @@ TcpLoop: if len(push.Mesh.OutboundClusterStatName) != 0 { statPrefix = telemetry.BuildStatPrefix(push.Mesh.OutboundClusterStatName, string(service.Hostname), "", &model.Port{Port: port}, 0, &service.Attributes) } + var destinationCIDRs []string + if destinationCIDR != "" { + destinationCIDRs = []string{destinationCIDR} + } out = append(out, &filterChainOpts{ destinationCIDRs: destinationCIDRs, networkFilters: lb.buildOutboundNetworkFiltersWithSingleDestination(statPrefix, clusterName, "", diff --git a/pilot/pkg/serviceregistry/serviceentry/controller.go b/pilot/pkg/serviceregistry/serviceentry/controller.go index 364a2aa13585..04683431f18d 100644 --- a/pilot/pkg/serviceregistry/serviceentry/controller.go +++ b/pilot/pkg/serviceregistry/serviceentry/controller.go @@ -362,7 +362,7 @@ func getUpdatedConfigs(services []*model.Service) sets.Set[model.ConfigKey] { func (s *Controller) serviceEntryHandler(old, curr config.Config, event model.Event) { log.Debugf("Handle event %s for service entry %s/%s", event, curr.Namespace, curr.Name) currentServiceEntry := curr.Spec.(*networking.ServiceEntry) - cs := convertServices(curr, s.clusterID) + cs := convertServices(curr) configsUpdated := sets.New[model.ConfigKey]() key := curr.NamespacedName() diff --git a/pilot/pkg/serviceregistry/serviceentry/controller_test.go b/pilot/pkg/serviceregistry/serviceentry/controller_test.go index 8cd125d30cf3..024ca898245f 100644 --- a/pilot/pkg/serviceregistry/serviceentry/controller_test.go +++ b/pilot/pkg/serviceregistry/serviceentry/controller_test.go @@ -129,10 +129,9 @@ func initServiceDiscoveryWithOpts(t test.Failer, workloadOnly bool, opts ...Opti func TestServiceDiscoveryServices(t *testing.T) { store, sd, fx := initServiceDiscovery(t) expectedServices := []*model.Service{ - makeService( - "*.istio.io", "httpDNSRR", []string{constants.UnspecifiedIP}, map[string]int{"http-port": 80, "http-alt-port": 8080}, true, model.DNSRoundRobinLB), - makeService("*.google.com", "httpDNS", []string{constants.UnspecifiedIP}, map[string]int{"http-port": 80, "http-alt-port": 8080}, true, model.DNSLB), - makeService("tcpstatic.com", "tcpStatic", []string{"172.217.0.1"}, map[string]int{"tcp-444": 444}, true, model.ClientSideLB), + makeService("*.istio.io", "httpDNSRR", constants.UnspecifiedIP, map[string]int{"http-port": 80, "http-alt-port": 8080}, true, model.DNSRoundRobinLB), + makeService("*.google.com", "httpDNS", constants.UnspecifiedIP, map[string]int{"http-port": 80, "http-alt-port": 8080}, true, model.DNSLB), + makeService("tcpstatic.com", "tcpStatic", "172.217.0.1", map[string]int{"tcp-444": 444}, true, model.ClientSideLB), } createConfigs([]*config.Config{httpDNS, httpDNSRR, tcpStatic}, store, t) @@ -1493,7 +1492,7 @@ func expectEvents(t testing.TB, ch *xdsfake.Updater, events ...Event) { func expectServiceInstances(t testing.TB, sd *Controller, cfg *config.Config, port int, expected ...[]*model.ServiceInstance) { t.Helper() - svcs := convertServices(*cfg, "") + svcs := convertServices(*cfg) if len(svcs) != len(expected) { t.Fatalf("got more services than expected: %v vs %v", len(svcs), len(expected)) } @@ -1727,8 +1726,8 @@ func TestServicesDiff(t *testing.T) { for _, tt := range cases { t.Run(tt.name, func(t *testing.T) { - as := convertServices(*tt.current, "") - bs := convertServices(*tt.new, "") + as := convertServices(*tt.current) + bs := convertServices(*tt.new) added, deleted, updated, unchanged := servicesDiff(as, bs) for i, item := range []struct { hostnames []host.Name diff --git a/pilot/pkg/serviceregistry/serviceentry/conversion.go b/pilot/pkg/serviceregistry/serviceentry/conversion.go index 5fa2e71fe007..43793940455c 100644 --- a/pilot/pkg/serviceregistry/serviceentry/conversion.go +++ b/pilot/pkg/serviceregistry/serviceentry/conversion.go @@ -17,6 +17,7 @@ package serviceentry import ( "net/netip" "strings" + "time" "istio.io/api/label" networking "istio.io/api/networking/v1alpha3" @@ -33,7 +34,6 @@ import ( "istio.io/istio/pkg/config/visibility" "istio.io/istio/pkg/kube/labels" "istio.io/istio/pkg/network" - "istio.io/istio/pkg/slices" "istio.io/istio/pkg/spiffe" netutil "istio.io/istio/pkg/util/net" "istio.io/istio/pkg/util/sets" @@ -48,8 +48,8 @@ func convertPort(port *networking.ServicePort) *model.Port { } type HostAddress struct { - host string - addresses []string + host string + address string } // ServiceToServiceEntry converts from internal Service representation to ServiceEntry @@ -139,7 +139,7 @@ func ServiceToServiceEntry(svc *model.Service, proxy *model.Proxy) *config.Confi } // convertServices transforms a ServiceEntry config to a list of internal Service objects. -func convertServices(cfg config.Config, clusterID cluster.ID) []*model.Service { +func convertServices(cfg config.Config) []*model.Service { serviceEntry := cfg.Spec.(*networking.ServiceEntry) creationTime := cfg.CreationTimestamp @@ -182,58 +182,56 @@ func convertServices(cfg config.Config, clusterID cluster.ID) []*model.Service { hostAddresses := []*HostAddress{} for _, hostname := range serviceEntry.Hosts { if len(serviceEntry.Addresses) > 0 { - ha := &HostAddress{hostname, []string{}} for _, address := range serviceEntry.Addresses { - // Check if addresses is an IP first because that is the most common case. + // Check if address is an IP first because that is the most common case. if netutil.IsValidIPAddress(address) { - ha.addresses = append(ha.addresses, address) + hostAddresses = append(hostAddresses, &HostAddress{hostname, address}) } else if cidr, cidrErr := netip.ParsePrefix(address); cidrErr == nil { newAddress := address if cidr.Bits() == cidr.Addr().BitLen() { - // /32 mask. Remove the /32 and make it a normal IP addresses + // /32 mask. Remove the /32 and make it a normal IP address newAddress = cidr.Addr().String() } - ha.addresses = append(ha.addresses, newAddress) + hostAddresses = append(hostAddresses, &HostAddress{hostname, newAddress}) } } - hostAddresses = append(hostAddresses, ha) } else { - hostAddresses = append(hostAddresses, &HostAddress{hostname, []string{constants.UnspecifiedIP}}) + hostAddresses = append(hostAddresses, &HostAddress{hostname, constants.UnspecifiedIP}) } } + return buildServices(hostAddresses, cfg.Name, cfg.Namespace, svcPorts, serviceEntry.Location, resolution, + exportTo, labelSelectors, serviceEntry.SubjectAltNames, creationTime, cfg.Labels, portOverrides) +} + +func buildServices(hostAddresses []*HostAddress, name, namespace string, ports model.PortList, location networking.ServiceEntry_Location, + resolution model.Resolution, exportTo sets.Set[visibility.Instance], selectors map[string]string, saccounts []string, + ctime time.Time, labels map[string]string, overrides map[uint32]uint32, +) []*model.Service { out := make([]*model.Service, 0, len(hostAddresses)) - lbls := cfg.Labels - if features.CanonicalServiceForMeshExternalServiceEntry && serviceEntry.Location == networking.ServiceEntry_MESH_EXTERNAL { - lbls = ensureCanonicalServiceLabels(cfg.Name, cfg.Labels) + lbls := labels + if features.CanonicalServiceForMeshExternalServiceEntry && location == networking.ServiceEntry_MESH_EXTERNAL { + lbls = ensureCanonicalServiceLabels(name, labels) } for _, ha := range hostAddresses { - svc := &model.Service{ - CreationTime: creationTime, - MeshExternal: serviceEntry.Location == networking.ServiceEntry_MESH_EXTERNAL, + out = append(out, &model.Service{ + CreationTime: ctime, + MeshExternal: location == networking.ServiceEntry_MESH_EXTERNAL, Hostname: host.Name(ha.host), - DefaultAddress: ha.addresses[0], - Ports: svcPorts, + DefaultAddress: ha.address, + Ports: ports, Resolution: resolution, Attributes: model.ServiceAttributes{ ServiceRegistry: provider.External, - PassthroughTargetPorts: portOverrides, + PassthroughTargetPorts: overrides, Name: ha.host, - Namespace: cfg.Namespace, + Namespace: namespace, Labels: lbls, ExportTo: exportTo, - LabelSelectors: labelSelectors, + LabelSelectors: selectors, }, - ServiceAccounts: serviceEntry.SubjectAltNames, - } - if !slices.Equal(ha.addresses, []string{constants.UnspecifiedIP}) { - svc.ClusterVIPs = model.AddressMap{ - Addresses: map[cluster.ID][]string{ - clusterID: ha.addresses, - }, - } - } - out = append(out, svc) + ServiceAccounts: saccounts, + }) } return out } @@ -328,7 +326,7 @@ func (s *Controller) convertServiceEntryToInstances(cfg config.Config, services return nil } if services == nil { - services = convertServices(cfg, s.clusterID) + services = convertServices(cfg) } for _, service := range services { for _, serviceEntryPort := range serviceEntry.Ports { @@ -424,7 +422,7 @@ func (s *Controller) convertWorkloadEntryToWorkloadInstance(cfg config.Config, c dnsServiceEntryOnly = true } if addr != "" && !netutil.IsValidIPAddress(addr) { - // k8s can't use workloads with hostnames in the addresses field. + // k8s can't use workloads with hostnames in the address field. dnsServiceEntryOnly = true } tlsMode := getTLSModeFromWorkloadEntry(we) diff --git a/pilot/pkg/serviceregistry/serviceentry/conversion_test.go b/pilot/pkg/serviceregistry/serviceentry/conversion_test.go index d7b8b75590ec..317562a1c882 100644 --- a/pilot/pkg/serviceregistry/serviceentry/conversion_test.go +++ b/pilot/pkg/serviceregistry/serviceentry/conversion_test.go @@ -35,7 +35,6 @@ import ( "istio.io/istio/pkg/config/protocol" "istio.io/istio/pkg/config/schema/gvk" "istio.io/istio/pkg/network" - "istio.io/istio/pkg/slices" "istio.io/istio/pkg/spiffe" "istio.io/istio/pkg/test" ) @@ -503,13 +502,13 @@ func convertPortNameToProtocol(name string) protocol.Instance { return protocol.Parse(prefix) } -func makeService(hostname host.Name, configNamespace string, addresses []string, ports map[string]int, +func makeService(hostname host.Name, configNamespace, address string, ports map[string]int, external bool, resolution model.Resolution, serviceAccounts ...string, ) *model.Service { svc := &model.Service{ CreationTime: GlobalTime, Hostname: hostname, - DefaultAddress: addresses[0], + DefaultAddress: address, MeshExternal: external, Resolution: resolution, ServiceAccounts: serviceAccounts, @@ -519,11 +518,6 @@ func makeService(hostname host.Name, configNamespace string, addresses []string, Namespace: configNamespace, }, } - if !slices.Equal(addresses, []string{constants.UnspecifiedIP}) { - svc.ClusterVIPs = model.AddressMap{ - Addresses: map[cluster.ID][]string{"": addresses}, - } - } if external && features.CanonicalServiceForMeshExternalServiceEntry { if svc.Attributes.Labels == nil { @@ -571,7 +565,7 @@ func makeInstanceWithServiceAccount(cfg *config.Config, address string, port int func makeTarget(cfg *config.Config, address string, port int, svcPort *networking.ServicePort, svcLabels map[string]string, mtlsMode MTLSMode, ) model.ServiceTarget { - services := convertServices(*cfg, "") + services := convertServices(*cfg) svc := services[0] // default for _, s := range services { if string(s.Hostname) == address { @@ -602,7 +596,7 @@ func makeTarget(cfg *config.Config, address string, port int, func makeInstance(cfg *config.Config, address string, port int, svcPort *networking.ServicePort, svcLabels map[string]string, mtlsMode MTLSMode, ) *model.ServiceInstance { - services := convertServices(*cfg, "") + services := convertServices(*cfg) svc := services[0] // default for _, s := range services { if string(s.Hostname) == address { @@ -655,7 +649,7 @@ func testConvertServiceBody(t *testing.T) { // service entry http externalSvc: httpNone, services: []*model.Service{ - makeService("*.google.com", "httpNone", []string{constants.UnspecifiedIP}, + makeService("*.google.com", "httpNone", constants.UnspecifiedIP, map[string]int{"http-number": 80, "http2-number": 8080}, true, model.Passthrough), }, }, @@ -663,7 +657,7 @@ func testConvertServiceBody(t *testing.T) { // service entry tcp externalSvc: tcpNone, services: []*model.Service{ - makeService("tcpnone.com", "tcpNone", []string{"172.217.0.0/16"}, + makeService("tcpnone.com", "tcpNone", "172.217.0.0/16", map[string]int{"tcp-444": 444}, true, model.Passthrough), }, }, @@ -671,7 +665,7 @@ func testConvertServiceBody(t *testing.T) { // service entry http static externalSvc: httpStatic, services: []*model.Service{ - makeService("*.google.com", "httpStatic", []string{constants.UnspecifiedIP}, + makeService("*.google.com", "httpStatic", constants.UnspecifiedIP, map[string]int{"http-port": 80, "http-alt-port": 8080}, true, model.ClientSideLB), }, }, @@ -679,9 +673,9 @@ func testConvertServiceBody(t *testing.T) { // service entry DNS with no endpoints externalSvc: httpDNSnoEndpoints, services: []*model.Service{ - makeService("google.com", "httpDNSnoEndpoints", []string{constants.UnspecifiedIP}, + makeService("google.com", "httpDNSnoEndpoints", constants.UnspecifiedIP, map[string]int{"http-port": 80, "http-alt-port": 8080}, true, model.DNSLB, "google.com"), - makeService("www.wikipedia.org", "httpDNSnoEndpoints", []string{constants.UnspecifiedIP}, + makeService("www.wikipedia.org", "httpDNSnoEndpoints", constants.UnspecifiedIP, map[string]int{"http-port": 80, "http-alt-port": 8080}, true, model.DNSLB, "google.com"), }, }, @@ -689,7 +683,7 @@ func testConvertServiceBody(t *testing.T) { // service entry dns externalSvc: httpDNS, services: []*model.Service{ - makeService("*.google.com", "httpDNS", []string{constants.UnspecifiedIP}, + makeService("*.google.com", "httpDNS", constants.UnspecifiedIP, map[string]int{"http-port": 80, "http-alt-port": 8080}, true, model.DNSLB), }, }, @@ -697,7 +691,7 @@ func testConvertServiceBody(t *testing.T) { // service entry dns with target port externalSvc: dnsTargetPort, services: []*model.Service{ - makeService("google.com", "dnsTargetPort", []string{constants.UnspecifiedIP}, + makeService("google.com", "dnsTargetPort", constants.UnspecifiedIP, map[string]int{"http-port": 80}, true, model.DNSLB), }, }, @@ -705,7 +699,7 @@ func testConvertServiceBody(t *testing.T) { // service entry tcp DNS externalSvc: tcpDNS, services: []*model.Service{ - makeService("tcpdns.com", "tcpDNS", []string{constants.UnspecifiedIP}, + makeService("tcpdns.com", "tcpDNS", constants.UnspecifiedIP, map[string]int{"tcp-444": 444}, true, model.DNSLB), }, }, @@ -713,7 +707,7 @@ func testConvertServiceBody(t *testing.T) { // service entry tcp static externalSvc: tcpStatic, services: []*model.Service{ - makeService("tcpstatic.com", "tcpStatic", []string{"172.217.0.1"}, + makeService("tcpstatic.com", "tcpStatic", "172.217.0.1", map[string]int{"tcp-444": 444}, true, model.ClientSideLB), }, }, @@ -721,7 +715,7 @@ func testConvertServiceBody(t *testing.T) { // service entry http internal externalSvc: httpNoneInternal, services: []*model.Service{ - makeService("*.google.com", "httpNoneInternal", []string{constants.UnspecifiedIP}, + makeService("*.google.com", "httpNoneInternal", constants.UnspecifiedIP, map[string]int{"http-number": 80, "http2-number": 8080}, false, model.Passthrough), }, }, @@ -729,7 +723,7 @@ func testConvertServiceBody(t *testing.T) { // service entry tcp internal externalSvc: tcpNoneInternal, services: []*model.Service{ - makeService("tcpinternal.com", "tcpNoneInternal", []string{"172.217.0.0/16"}, + makeService("tcpinternal.com", "tcpNoneInternal", "172.217.0.0/16", map[string]int{"tcp-444": 444}, false, model.Passthrough), }, }, @@ -737,15 +731,19 @@ func testConvertServiceBody(t *testing.T) { // service entry multiAddrInternal externalSvc: multiAddrInternal, services: []*model.Service{ - makeService("tcp1.com", "multiAddrInternal", []string{"1.1.1.0/16", "2.2.2.0/16"}, + makeService("tcp1.com", "multiAddrInternal", "1.1.1.0/16", + map[string]int{"tcp-444": 444}, false, model.Passthrough), + makeService("tcp1.com", "multiAddrInternal", "2.2.2.0/16", + map[string]int{"tcp-444": 444}, false, model.Passthrough), + makeService("tcp2.com", "multiAddrInternal", "1.1.1.0/16", map[string]int{"tcp-444": 444}, false, model.Passthrough), - makeService("tcp2.com", "multiAddrInternal", []string{"1.1.1.0/16", "2.2.2.0/16"}, + makeService("tcp2.com", "multiAddrInternal", "2.2.2.0/16", map[string]int{"tcp-444": 444}, false, model.Passthrough), }, }, } - selectorSvc := makeService("selector.com", "selector", []string{constants.UnspecifiedIP}, + selectorSvc := makeService("selector.com", "selector", "0.0.0.0", map[string]int{"tcp-444": 444, "http-445": 445}, true, model.ClientSideLB) selectorSvc.Attributes.LabelSelectors = map[string]string{"app": "wle"} @@ -758,7 +756,7 @@ func testConvertServiceBody(t *testing.T) { }) for _, tt := range serviceTests { - services := convertServices(*tt.externalSvc, "") + services := convertServices(*tt.externalSvc) if err := compare(t, services, tt.services); err != nil { t.Errorf("testcase: %v\n%v ", tt.externalSvc.Name, err) } @@ -945,7 +943,7 @@ func TestConvertWorkloadEntryToServiceInstances(t *testing.T) { for _, tt := range serviceInstanceTests { t.Run(tt.name, func(t *testing.T) { - services := convertServices(*tt.se, "") + services := convertServices(*tt.se) s := &Controller{} instances := s.convertWorkloadEntryToServiceInstances(tt.wle, services, tt.se.Spec.(*networking.ServiceEntry), &configKey{}, tt.clusterID) sortServiceInstances(instances) diff --git a/pilot/pkg/serviceregistry/serviceentry/store_test.go b/pilot/pkg/serviceregistry/serviceentry/store_test.go index 1ab17b833c48..850242ddf120 100644 --- a/pilot/pkg/serviceregistry/serviceentry/store_test.go +++ b/pilot/pkg/serviceregistry/serviceentry/store_test.go @@ -114,9 +114,8 @@ func TestServiceStore(t *testing.T) { } expectedServices := []*model.Service{ - makeService( - "*.istio.io", "httpDNSRR", []string{constants.UnspecifiedIP}, map[string]int{"http-port": 80, "http-alt-port": 8080}, true, model.DNSRoundRobinLB), - makeService("*.istio.io", "httpDNSRR", []string{constants.UnspecifiedIP}, map[string]int{"http-port": 80, "http-alt-port": 8080}, true, model.DNSLB), + makeService("*.istio.io", "httpDNSRR", constants.UnspecifiedIP, map[string]int{"http-port": 80, "http-alt-port": 8080}, true, model.DNSRoundRobinLB), + makeService("*.istio.io", "httpDNSRR", constants.UnspecifiedIP, map[string]int{"http-port": 80, "http-alt-port": 8080}, true, model.DNSLB), } store.updateServices(httpDNSRR.NamespacedName(), expectedServices) diff --git a/pilot/pkg/xds/eds_test.go b/pilot/pkg/xds/eds_test.go index a8b30d2c4d7e..595cc2e17cef 100644 --- a/pilot/pkg/xds/eds_test.go +++ b/pilot/pkg/xds/eds_test.go @@ -42,7 +42,6 @@ import ( xdsfake "istio.io/istio/pilot/test/xds" "istio.io/istio/pilot/test/xdstest" "istio.io/istio/pkg/adsc" - "istio.io/istio/pkg/config/constants" "istio.io/istio/pkg/config/host" "istio.io/istio/pkg/config/protocol" "istio.io/istio/pkg/config/schema/kind" @@ -1231,8 +1230,7 @@ func updateServiceResolution(s *xdsfake.FakeDiscoveryServer, resolution model.Re func addOverlappingEndpoints(s *xdsfake.FakeDiscoveryServer) { svc := &model.Service{ - DefaultAddress: constants.UnspecifiedIP, - Hostname: "overlapping.cluster.local", + Hostname: "overlapping.cluster.local", Ports: model.PortList{ { Name: "dns", @@ -1265,8 +1263,7 @@ func addOverlappingEndpoints(s *xdsfake.FakeDiscoveryServer) { func addUnhealthyCluster(s *xdsfake.FakeDiscoveryServer) { svc := &model.Service{ - DefaultAddress: constants.UnspecifiedIP, - Hostname: "unhealthy.svc.cluster.local", + Hostname: "unhealthy.svc.cluster.local", Ports: model.PortList{ { Name: "tcp-dns", diff --git a/releasenotes/notes/51967-revert.yaml b/releasenotes/notes/51967-revert.yaml new file mode 100644 index 000000000000..3442b412869d --- /dev/null +++ b/releasenotes/notes/51967-revert.yaml @@ -0,0 +1,9 @@ +apiVersion: release-notes/v2 +kind: bug-fix +area: traffic-management +issue: + - 52944 + - 52847 +releaseNotes: + - | + **Removed** a change in 1.22.4 to the handling of multiple service VIPs in ServiceEntry. diff --git a/releasenotes/notes/51967.yaml b/releasenotes/notes/51967.yaml deleted file mode 100644 index 0ccbf6ecc2d5..000000000000 --- a/releasenotes/notes/51967.yaml +++ /dev/null @@ -1,8 +0,0 @@ -apiVersion: release-notes/v2 -kind: bug-fix -area: traffic-management -issue: - - 51747 -releaseNotes: - - | - **Fixed** matching multiple service VIPs in ServiceEntry. diff --git a/tests/integration/pilot/common/routing.go b/tests/integration/pilot/common/routing.go index 22aa1389caac..ac70fffec317 100644 --- a/tests/integration/pilot/common/routing.go +++ b/tests/integration/pilot/common/routing.go @@ -18,7 +18,6 @@ package common import ( - "context" "fmt" "net/http" "net/netip" @@ -30,12 +29,7 @@ import ( "google.golang.org/grpc/codes" wrappers "google.golang.org/protobuf/types/known/wrapperspb" - corev1 "k8s.io/api/core/v1" - kerrors "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/util/intstr" - "istio.io/api/annotation" "istio.io/istio/pilot/pkg/model" "istio.io/istio/pkg/config/host" "istio.io/istio/pkg/config/protocol" @@ -53,7 +47,6 @@ import ( "istio.io/istio/pkg/test/framework/components/istio/ingress" "istio.io/istio/pkg/test/framework/label" "istio.io/istio/pkg/test/scopes" - "istio.io/istio/pkg/test/util/retry" "istio.io/istio/pkg/test/util/tmpl" "istio.io/istio/pkg/util/sets" "istio.io/istio/tests/common/jwt" @@ -3860,67 +3853,6 @@ spec: } } -func testServiceEntryWithMultipleVIPsAndResolutionNone(t TrafficContext) { - if len(t.Apps.External.All) == 0 { - t.Skip("no external service instances") - } - - clusterIPs := createService(t, "external", t.Apps.External.All.NamespaceName(), "external", 2) - if len(clusterIPs) < 2 { - t.Errorf("failed to get 2 cluster IPs, got %v", clusterIPs) - } - - // Create CIDRs from IPs. We want to check if CIDR matching in a wildcard listener works. - var cidrs []string - for _, clusterIP := range clusterIPs { - if ip, err := netip.ParseAddr(clusterIP); err != nil { - t.Errorf("failed to parse cluster IP address '%s': %s", err) - } else if ip.Is4() { - cidrs = append(cidrs, fmt.Sprintf("%s/24", clusterIP)) - } else if ip.Is6() { - cidrs = append(cidrs, clusterIP) - } - } - - serviceEntry := tmpl.MustEvaluate(` -apiVersion: networking.istio.io/v1alpha3 -kind: ServiceEntry -metadata: - name: server-default-svc - namespace: {{.Namespace}} -spec: - exportTo: - - "." - hosts: - - server.default.svc - addresses: -{{ range $ip := .IPs }} - - "{{$ip}}" -{{ end }} - location: MESH_EXTERNAL - ports: - - number: 443 - name: tcp - protocol: TCP - resolution: NONE -`, map[string]any{"Namespace": t.Apps.A.NamespaceName(), "IPs": cidrs}) - - t.RunTraffic(TrafficTestCase{ - name: "send a request to one of the service entry VIPs", - config: serviceEntry, - opts: echo.CallOptions{ - // Use second IP address to make sure that Istio matches all CIDRs, not only the first one. - Address: clusterIPs[1], - Port: echo.Port{ - Protocol: protocol.HTTPS, - ServicePort: 443, - }, - Check: check.Status(http.StatusOK), - }, - call: t.Apps.A[0].CallOrFail, - }) -} - func destinationRule(app, mode string) string { return fmt.Sprintf(`apiVersion: networking.istio.io/v1beta1 kind: DestinationRule @@ -5090,59 +5022,3 @@ func LocationHeader(expected string) echo.Checker { "Location") }) } - -// createService creates additional Services for a given app to obtain routable VIPs inside the cluster. -func createService(t TrafficContext, name, ns, appLabelValue string, instances int) []string { - var clusterIPs []string - for i := 0; i < instances; i++ { - svcName := fmt.Sprintf("%s-vip-%d", name, i) - svc := &corev1.Service{ - ObjectMeta: metav1.ObjectMeta{ - Name: svcName, - Namespace: ns, - Annotations: map[string]string{ - // Export the service nowhere, so that no proxy will receive it or its VIP. - annotation.NetworkingExportTo.Name: "~", - }, - }, - Spec: corev1.ServiceSpec{ - Selector: map[string]string{ - "app": appLabelValue, - }, - Type: corev1.ServiceTypeClusterIP, - }, - } - for _, p := range ports.All() { - if p.ServicePort == echo.NoServicePort { - continue - } - svc.Spec.Ports = append(svc.Spec.Ports, corev1.ServicePort{ - Name: p.Name, - Protocol: corev1.ProtocolTCP, - Port: int32(p.ServicePort), - TargetPort: intstr.IntOrString{IntVal: int32(ports.HTTPS.WorkloadPort)}, - }) - } - _, err := t.Clusters().Default().Kube().CoreV1().Services(ns).Create(context.TODO(), svc, metav1.CreateOptions{}) - if err != nil && !kerrors.IsAlreadyExists(err) { - t.Errorf("failed to create service %s: %s", svc, err) - } - - // Wait until a ClusterIP has been assigned. - err = retry.UntilSuccess(func() error { - svc, err := t.Clusters().Default().Kube().CoreV1().Services(ns).Get(context.TODO(), svcName, metav1.GetOptions{}) - if err != nil { - return err - } - if len(svc.Spec.ClusterIPs) == 0 { - return fmt.Errorf("service VIP not set for service") - } - clusterIPs = append(clusterIPs, svc.Spec.ClusterIPs...) - return nil - }, retry.Timeout(10*time.Second)) - if err != nil { - t.Errorf("failed to assign ClusterIP for %s service: %s", svcName, err) - } - } - return clusterIPs -} diff --git a/tests/integration/pilot/common/traffic.go b/tests/integration/pilot/common/traffic.go index e47919f1c425..5d5601409c33 100644 --- a/tests/integration/pilot/common/traffic.go +++ b/tests/integration/pilot/common/traffic.go @@ -297,7 +297,6 @@ func RunAllTrafficTests(t framework.TestContext, i istio.Instance, apps deployme RunCase("vm", VMTestCases(apps.VM)) RunSkipAmbient("dns", DNSTestCases, "https://github.com/istio/istio/issues/48614") RunCase("externalservice", TestExternalService) - RunSkipAmbient("service-entry-vips-resolution-none", testServiceEntryWithMultipleVIPsAndResolutionNone, "") } func ExpectString(got, expected, help string) error { diff --git a/tests/integration/pilot/dns_auto_allocation_test.go b/tests/integration/pilot/dns_auto_allocation_test.go deleted file mode 100644 index 568b94ed4319..000000000000 --- a/tests/integration/pilot/dns_auto_allocation_test.go +++ /dev/null @@ -1,104 +0,0 @@ -//go:build integ -// +build integ - -// Copyright Istio Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package pilot - -import ( - "fmt" - "testing" - "time" - - "istio.io/istio/pkg/config/protocol" - "istio.io/istio/pkg/test/framework" - "istio.io/istio/pkg/test/framework/components/echo" - "istio.io/istio/pkg/test/framework/components/echo/check" - "istio.io/istio/pkg/test/framework/components/echo/deployment" - "istio.io/istio/pkg/test/framework/components/namespace" - "istio.io/istio/pkg/test/util/retry" -) - -func TestDNSAutoAllocation(t *testing.T) { - framework. - NewTest(t). - Run(func(t framework.TestContext) { - ns := namespace.NewOrFail(t, t, namespace.Config{ - Prefix: "dns-auto-allocation", - Inject: true, - }) - cfg := `apiVersion: networking.istio.io/v1beta1 -kind: ProxyConfig -metadata: - name: enable-dns-auto-allocation -spec: - selector: - matchLabels: - app: a - environmentVariables: - ISTIO_META_DNS_CAPTURE: "true" - ISTIO_META_DNS_AUTO_ALLOCATE: "true" ---- -apiVersion: networking.istio.io/v1alpha3 -kind: ServiceEntry -metadata: - name: fake-local -spec: - hosts: - - "fake.local" - resolution: DNS - ports: - - number: 80 - name: http - protocol: HTTP ---- -apiVersion: networking.istio.io/v1alpha3 -kind: VirtualService -metadata: - name: route-to-b -spec: - hosts: - - fake.local - http: - - route: - - destination: - host: b.{{.echoNamespace}}.svc.cluster.local -` - t.ConfigIstio().Eval(ns.Name(), map[string]string{"echoNamespace": apps.Namespace.Name()}, cfg).ApplyOrFail(t) - instances := deployment.New(t, t.AllClusters().Configs()...).WithConfig(echo.Config{Namespace: ns, Service: "a"}).BuildOrFail(t) - - retry.UntilSuccessOrFail(t, func() error { - _, err := instances[0].Call(echo.CallOptions{ - Address: "fake.local", - Port: echo.Port{ - Name: "http", - ServicePort: 80, - Protocol: protocol.HTTP, - }, - Check: check.OK(), - }) - if err == nil { - return nil - } - // trigger injection in case of delay of ProxyConfig propagation - for _, i := range instances { - if err := i.Restart(); err != nil { - return fmt.Errorf("failed to restart echo instance: %v", err) - } - } - return nil - }, retry.Timeout(time.Second*30)) - }) -} From 789be11de7433de8bcc6c100e0a1b7178f65ca87 Mon Sep 17 00:00:00 2001 From: Istio Automation Date: Wed, 4 Sep 2024 06:46:27 -0400 Subject: [PATCH 3/7] [release-1.22] fix analyze vs trriger panic when spec.routes.destinations[].destination null (#52992) * fix analyze vs trriger panic when spec.routes.destinations[].destination null * add analyze test --------- Co-authored-by: nicole-lihui --- .../analysis/analyzers/analyzers_test.go | 1 + .../virtualservice_destinationhosts.yaml | 1 + .../analysis/analyzers/util/service_lookup.go | 4 ++-- .../virtualservice/destinationhosts.go | 22 +++++++++---------- 4 files changed, 15 insertions(+), 13 deletions(-) diff --git a/pkg/config/analysis/analyzers/analyzers_test.go b/pkg/config/analysis/analyzers/analyzers_test.go index eef07c581484..4bc314e81e90 100644 --- a/pkg/config/analysis/analyzers/analyzers_test.go +++ b/pkg/config/analysis/analyzers/analyzers_test.go @@ -422,6 +422,7 @@ var testGrid = []testCase{ inputFiles: []string{"testdata/virtualservice_destinationhosts.yaml"}, analyzer: &virtualservice.DestinationHostAnalyzer{}, expected: []message{ + {msg.ReferencedResourceNotFound, "VirtualService default/reviews-bogushost"}, {msg.ReferencedResourceNotFound, "VirtualService default/reviews-bogushost"}, {msg.ReferencedResourceNotFound, "VirtualService default/reviews-bookinfo-other"}, {msg.ReferencedResourceNotFound, "VirtualService default/reviews-mirror-bogushost"}, diff --git a/pkg/config/analysis/analyzers/testdata/virtualservice_destinationhosts.yaml b/pkg/config/analysis/analyzers/testdata/virtualservice_destinationhosts.yaml index d81ee84038c1..5601eb6ebf3b 100644 --- a/pkg/config/analysis/analyzers/testdata/virtualservice_destinationhosts.yaml +++ b/pkg/config/analysis/analyzers/testdata/virtualservice_destinationhosts.yaml @@ -94,6 +94,7 @@ metadata: spec: http: - route: + - {} # test destination null - destination: host: reviews-bogus # This host does not exist, should result in a validation error subset: v1 diff --git a/pkg/config/analysis/analyzers/util/service_lookup.go b/pkg/config/analysis/analyzers/util/service_lookup.go index e2cdbb68a6c2..ef95449289f6 100644 --- a/pkg/config/analysis/analyzers/util/service_lookup.go +++ b/pkg/config/analysis/analyzers/util/service_lookup.go @@ -36,12 +36,12 @@ func InitServiceEntryHostMap(ctx analysis.Context) map[ScopedFqdn]*v1alpha3.Serv exportsToAll := false for _, h := range s.GetHosts() { // ExportToAll scenario - if len(s.ExportTo) == 0 || exportsToAll { + if len(s.GetExportTo()) == 0 || exportsToAll { result[NewScopedFqdn(ExportToAllNamespaces, r.Metadata.FullName.Namespace, h)] = s continue // If exports to all, we can skip adding to each namespace } - for _, ns := range s.ExportTo { + for _, ns := range s.GetExportTo() { switch ns { case ExportToAllNamespaces: result[NewScopedFqdn(ExportToAllNamespaces, r.Metadata.FullName.Namespace, h)] = s diff --git a/pkg/config/analysis/analyzers/virtualservice/destinationhosts.go b/pkg/config/analysis/analyzers/virtualservice/destinationhosts.go index a3edea422a0d..9b63917ed5e9 100644 --- a/pkg/config/analysis/analyzers/virtualservice/destinationhosts.go +++ b/pkg/config/analysis/analyzers/virtualservice/destinationhosts.go @@ -66,16 +66,16 @@ func (a *DestinationHostAnalyzer) analyzeSubset(r *resource.Instance, ctx analys vs := r.Message.(*v1alpha3.VirtualService) // if there's no gateway specified, we're done - if len(vs.Gateways) == 0 { + if len(vs.GetGateways()) == 0 { return } - for ruleIndex, http := range vs.Http { - for routeIndex, route := range http.Route { - if route.Destination.Subset == "" { + for ruleIndex, http := range vs.GetHttp() { + for routeIndex, route := range http.GetRoute() { + if route.GetDestination().GetSubset() == "" { for virtualservice, destinations := range vsDestinations { for _, destination := range destinations { - if destination.Host == route.Destination.Host { + if destination.GetHost() == route.GetDestination().GetHost() { m := msg.NewIngressRouteRulesNotAffected(r, virtualservice.String(), r.Metadata.FullName.String()) key := fmt.Sprintf(util.DestinationHost, http.Name, ruleIndex, routeIndex) @@ -98,13 +98,13 @@ func initVirtualServiceDestinations(ctx analysis.Context) map[resource.FullName] ctx.ForEach(gvk.VirtualService, func(r *resource.Instance) bool { virtualservice := r.Message.(*v1alpha3.VirtualService) - for _, routes := range virtualservice.Http { - for _, destinations := range routes.Route { + for _, routes := range virtualservice.GetHttp() { + for _, destinations := range routes.GetRoute() { // if there's no subset specified, we're done - if destinations.Destination.Subset != "" { - for _, host := range virtualservice.Hosts { - if destinations.Destination.Host == host { - virtualservices[r.Metadata.FullName] = append(virtualservices[r.Metadata.FullName], destinations.Destination) + if destinations.GetDestination().GetSubset() != "" { + for _, host := range virtualservice.GetHosts() { + if destinations.GetDestination().GetHost() == host { + virtualservices[r.Metadata.FullName] = append(virtualservices[r.Metadata.FullName], destinations.GetDestination()) } } } From 32f1d50a97bcfbdebfae6635aa3fb385a4b5acde Mon Sep 17 00:00:00 2001 From: Istio Automation Date: Wed, 4 Sep 2024 15:45:27 -0400 Subject: [PATCH 4/7] Update BASE_VERSION to 1.22-2024-09-04T19-02-08 (#52997) --- Makefile.core.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile.core.mk b/Makefile.core.mk index 62228b1455d8..ccb7a4d65467 100644 --- a/Makefile.core.mk +++ b/Makefile.core.mk @@ -49,7 +49,7 @@ endif export VERSION # Base version of Istio image to use -BASE_VERSION ?= 1.22-2024-08-08T19-01-18 +BASE_VERSION ?= 1.22-2024-09-04T19-02-08 ISTIO_BASE_REGISTRY ?= gcr.io/istio-release export GO111MODULE ?= on From d954c5c1654a490bd0deeab64a098a691c1a1fa5 Mon Sep 17 00:00:00 2001 From: Istio Automation Date: Thu, 5 Sep 2024 03:54:27 -0400 Subject: [PATCH 5/7] fix endpoint comparision logic in UpdateServiceEndpoints (#52963) (#53005) Co-authored-by: Srinivas Surishetty --- pilot/pkg/model/service.go | 2 +- .../serviceregistry/serviceregistry_test.go | 103 ++++++++++++++++++ 2 files changed, 104 insertions(+), 1 deletion(-) diff --git a/pilot/pkg/model/service.go b/pilot/pkg/model/service.go index ca75b08e8068..19e9a75efacd 100644 --- a/pilot/pkg/model/service.go +++ b/pilot/pkg/model/service.go @@ -587,7 +587,7 @@ func (ep *IstioEndpoint) CmpOpts() []cmp.Option { // Key returns a function suitable for usage to distinguish this IstioEndpoint from another func (ep *IstioEndpoint) Key() string { - return ep.Address + "/" + ep.WorkloadName + return ep.Address + "/" + ep.WorkloadName + "/" + ep.ServicePortName } // EndpointMetadata represents metadata set on Envoy LbEndpoint used for telemetry purposes. diff --git a/pilot/pkg/serviceregistry/serviceregistry_test.go b/pilot/pkg/serviceregistry/serviceregistry_test.go index 750345639310..2ff87c16ec74 100644 --- a/pilot/pkg/serviceregistry/serviceregistry_test.go +++ b/pilot/pkg/serviceregistry/serviceregistry_test.go @@ -1125,6 +1125,109 @@ func TestWorkloadInstances(t *testing.T) { expectServiceEndpoints(t, fx, expectedSvc2, 80, instances) }) + t.Run("Multiport ServiceEntry selects WorkloadEntry", func(t *testing.T) { + store, _, fx := setupTest(t) + makeIstioObject(t, store, config.Config{ + Meta: config.Meta{ + Name: "service-entry", + Namespace: namespace, + GroupVersionKind: gvk.ServiceEntry, + Domain: "cluster.local", + }, + Spec: &networking.ServiceEntry{ + Hosts: []string{"service.namespace.svc.cluster.local"}, + Ports: []*networking.ServicePort{ + { + Name: "http", + Number: 80, + Protocol: "http", + }, + { + Name: "tcp", + Number: 3000, + Protocol: "tcp", + }, + }, + WorkloadSelector: &networking.WorkloadSelector{ + Labels: map[string]string{ + "app.foo": "true", + }, + }, + Resolution: networking.ServiceEntry_STATIC, + }, + }) + makeIstioObject(t, store, config.Config{ + Meta: config.Meta{ + Name: "workloadentry", + Namespace: namespace, + GroupVersionKind: gvk.WorkloadEntry, + Domain: "cluster.local", + }, + Spec: &networking.WorkloadEntry{ + Address: "2.3.4.5", + Labels: map[string]string{ + "app.foo": "true", + }, + Ports: map[string]uint32{ + "http": 8080, + "tcp": 3000, + }, + }, + }) + + fx.WaitOrFail(t, "xds full") + expectedSvc := &model.Service{ + Hostname: "service.namespace.svc.cluster.local", + Ports: []*model.Port{ + { + Name: "http", + Port: 80, + Protocol: "http", + }, + { + Name: "tcp", + Port: 3000, + Protocol: "tcp", + }, + }, + Attributes: model.ServiceAttributes{ + Namespace: namespace, + Name: "service-entry", + LabelSelectors: map[string]string{ + "app.foo": "true", + }, + }, + } + // Two endpoints will be generated: one for service port 80 and another for service port 3000. + expectServiceEndpoints(t, fx, expectedSvc, 80, []EndpointResponse{{Address: "2.3.4.5", Port: 8080}}) + expectServiceEndpoints(t, fx, expectedSvc, 3000, []EndpointResponse{{Address: "2.3.4.5", Port: 3000}}) + fx.Clear() + // Update the TCP port to 4000. + // This update should trigger a comparison with the TCP endpoint, not the HTTP one, + // which could cause unnecessary pushes. + makeIstioObject(t, store, config.Config{ + Meta: config.Meta{ + Name: "workloadentry", + Namespace: namespace, + GroupVersionKind: gvk.WorkloadEntry, + Domain: "cluster.local", + }, + Spec: &networking.WorkloadEntry{ + Address: "2.3.4.5", + Labels: map[string]string{ + "app.foo": "true", + }, + Ports: map[string]uint32{ + "http": 8080, + "tcp": 4000, + }, + }, + }) + fx.WaitOrFail(t, "xds") + expectServiceEndpoints(t, fx, expectedSvc, 80, []EndpointResponse{{Address: "2.3.4.5", Port: 8080}}) + expectServiceEndpoints(t, fx, expectedSvc, 3000, []EndpointResponse{{Address: "2.3.4.5", Port: 4000}}) + }) + t.Run("All directions", func(t *testing.T) { store, kube, fx := setupTest(t) makeService(t, kube, service) From 3ba7ca6bbefb352235f4604b4dbc949d39de5006 Mon Sep 17 00:00:00 2001 From: Ingwon Song <102102227+ingwonsong@users.noreply.github.com> Date: Sat, 7 Sep 2024 20:35:30 -0700 Subject: [PATCH 6/7] Return nil for io.EOF (#52833) (#53044) Change-Id: I88e225acad5a359f663eaf6b215c601b13410ea0 --- pilot/pkg/grpc/grpc.go | 30 +++++++++++++++++++++--------- pilot/pkg/grpc/grpc_test.go | 26 +++++++++++++++++++++++--- pilot/pkg/xds/ads.go | 2 +- pilot/pkg/xds/delta.go | 2 +- pkg/istio-agent/xds_proxy.go | 16 ++++++++++++---- 5 files changed, 58 insertions(+), 18 deletions(-) diff --git a/pilot/pkg/grpc/grpc.go b/pilot/pkg/grpc/grpc.go index 053ce3d97064..b54340c42e04 100644 --- a/pilot/pkg/grpc/grpc.go +++ b/pilot/pkg/grpc/grpc.go @@ -102,28 +102,40 @@ func containsExpectedMessage(msg string) bool { return false } -// IsExpectedGRPCError checks a gRPC error code and determines whether it is an expected error when -// things are operating normally. This is basically capturing when the client disconnects. -func IsExpectedGRPCError(err error) bool { +type ErrorType string + +const ( + // This indicates all the errors except the expected errors or graceful termination. + UnexpectedError ErrorType = "unexpectedError" + // This indicates an expected error when things are operating normally. + ExpectedError ErrorType = "expectedError" + // This indicates an error which happen when the connection is gracefully terminated. + // For example, the peer calls `SendClose()`. + GracefulTermination ErrorType = "gracefulTermination" +) + +// GRPCErrorType checks a gRPC error code and determines its ErrorType. +// This is basically capturing when the peer disconnects. +func GRPCErrorType(err error) ErrorType { if err == io.EOF { - return true + return GracefulTermination } if s, ok := status.FromError(err); ok { if s.Code() == codes.Canceled || s.Code() == codes.DeadlineExceeded { - return true + return ExpectedError } if s.Code() == codes.Unavailable && containsExpectedMessage(s.Message()) { - return true + return ExpectedError } } // If this is not a gRPCStatus we should just error message. if strings.Contains(err.Error(), "stream terminated by RST_STREAM with error code: NO_ERROR") { - return true + return ExpectedError } if strings.Contains(err.Error(), "received prior goaway: code: NO_ERROR") { - return true + return ExpectedError } - return false + return ExpectedError } diff --git a/pilot/pkg/grpc/grpc_test.go b/pilot/pkg/grpc/grpc_test.go index af2070d45b52..6bed91f040c7 100644 --- a/pilot/pkg/grpc/grpc_test.go +++ b/pilot/pkg/grpc/grpc_test.go @@ -16,12 +16,32 @@ package grpc import ( "errors" + "io" "testing" ) func TestIsExpectedGRPCError(t *testing.T) { - err := errors.New("code = Internal desc = stream terminated by RST_STREAM with error code: NO_ERROR") - if got := IsExpectedGRPCError(err); !got { - t.Fatalf("expected true, got %v", got) + tests := []struct { + name string + err error + want ErrorType + }{ + { + name: "RST_STREAM", + err: errors.New("code = Internal desc = stream terminated by RST_STREAM with error code: NO_ERROR"), + want: ExpectedError, + }, + { + name: "EOF", + err: io.EOF, + want: GracefulTermination, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := GRPCErrorType(tc.err); got != tc.want { + t.Fatalf("expected got %v, but want: %v", got, tc.want) + } + }) } } diff --git a/pilot/pkg/xds/ads.go b/pilot/pkg/xds/ads.go index 22ece010c991..0032e0572a47 100644 --- a/pilot/pkg/xds/ads.go +++ b/pilot/pkg/xds/ads.go @@ -164,7 +164,7 @@ func (s *DiscoveryServer) receive(con *Connection, identities []string) { for { req, err := con.stream.Recv() if err != nil { - if istiogrpc.IsExpectedGRPCError(err) { + if istiogrpc.GRPCErrorType(err) != istiogrpc.UnexpectedError { log.Infof("ADS: %q %s terminated", con.peerAddr, con.conID) return } diff --git a/pilot/pkg/xds/delta.go b/pilot/pkg/xds/delta.go index 9c8441a1b3f1..a90fcee61af1 100644 --- a/pilot/pkg/xds/delta.go +++ b/pilot/pkg/xds/delta.go @@ -200,7 +200,7 @@ func (s *DiscoveryServer) receiveDelta(con *Connection, identities []string) { for { req, err := con.deltaStream.Recv() if err != nil { - if istiogrpc.IsExpectedGRPCError(err) { + if istiogrpc.GRPCErrorType(err) != istiogrpc.UnexpectedError { deltaLog.Infof("ADS: %q %s terminated", con.peerAddr, con.conID) return } diff --git a/pkg/istio-agent/xds_proxy.go b/pkg/istio-agent/xds_proxy.go index 8cb3fa3676b0..e7118381ddd2 100644 --- a/pkg/istio-agent/xds_proxy.go +++ b/pkg/istio-agent/xds_proxy.go @@ -830,10 +830,14 @@ func (p *XdsProxy) initDebugInterface(port int) error { // upstreamErr sends the error to upstreamError channel, and return immediately if the connection closed. func upstreamErr(con *ProxyConnection, err error) { - if istiogrpc.IsExpectedGRPCError(err) { + switch istiogrpc.GRPCErrorType(err) { + case istiogrpc.GracefulTermination: + err = nil + fallthrough + case istiogrpc.ExpectedError: proxyLog.WithLabels("id", con.conID).Debugf("upstream terminated with status %v", err) metrics.IstiodConnectionCancellations.Increment() - } else { + default: proxyLog.WithLabels("id", con.conID).Warnf("upstream terminated with unexpected error %v", err) metrics.IstiodConnectionErrors.Increment() } @@ -845,10 +849,14 @@ func upstreamErr(con *ProxyConnection, err error) { // downstreamErr sends the error to downstreamError channel, and return immediately if the connection closed. func downstreamErr(con *ProxyConnection, err error) { - if istiogrpc.IsExpectedGRPCError(err) { + switch istiogrpc.GRPCErrorType(err) { + case istiogrpc.GracefulTermination: + err = nil + fallthrough + case istiogrpc.ExpectedError: proxyLog.WithLabels("id", con.conID).Debugf("downstream terminated with status %v", err) metrics.EnvoyConnectionCancellations.Increment() - } else { + default: proxyLog.WithLabels("id", con.conID).Warnf("downstream terminated with unexpected error %v", err) metrics.EnvoyConnectionErrors.Increment() } From f57a44a5bcbbba4cae93c872c30e361592a2129f Mon Sep 17 00:00:00 2001 From: "Jackie Maertens (Elliott)" <64559656+jaellio@users.noreply.github.com> Date: Tue, 17 Sep 2024 11:49:17 -0700 Subject: [PATCH 7/7] [release-1.22] Update proxy repo SHA for release-1.22.5 (#140) * [release-1.22] Update proxy repo SHA for release-1.22.5 Signed-off-by: Jackie Elliott * Update SHA Signed-off-by: Jackie Elliott * Add unit test for setting runtimeValues in Envoy config Signed-off-by: Jackie Elliott * Run make gen and fix explicit internal address test json Signed-off-by: Jackie Elliott * Explicitly set internal addresses in http connection manager when PILOT_SIDECAR_USE_REMOTE_ADDRESS is set to true for the sidecar on outbound. Signed-off-by: Jackie Elliott --------- Signed-off-by: Jackie Elliott --- istio.deps | 2 +- pilot/pkg/networking/core/listener_builder.go | 5 +- .../networking/core/listener_builder_test.go | 69 +++ pkg/bootstrap/instance_test.go | 3 + .../explicit_internal_address.proxycfg | 12 + .../explicit_internal_address_golden.json | 429 ++++++++++++++++++ 6 files changed, 518 insertions(+), 2 deletions(-) create mode 100644 pkg/bootstrap/testdata/explicit_internal_address.proxycfg create mode 100644 pkg/bootstrap/testdata/explicit_internal_address_golden.json diff --git a/istio.deps b/istio.deps index 01213541ccff..0a09397889f7 100644 --- a/istio.deps +++ b/istio.deps @@ -4,7 +4,7 @@ "name": "PROXY_REPO_SHA", "repoName": "proxy", "file": "", - "lastStableSHA": "41f0598c8d4514c4c7e17b5f56a064efdce0bf9d" + "lastStableSHA": "f190ab5d663a808ed8e4fea58eba4447cd51f2a7" }, { "_comment": "", diff --git a/pilot/pkg/networking/core/listener_builder.go b/pilot/pkg/networking/core/listener_builder.go index c05c26c39a90..67db37d9224f 100644 --- a/pilot/pkg/networking/core/listener_builder.go +++ b/pilot/pkg/networking/core/listener_builder.go @@ -421,7 +421,10 @@ func (lb *ListenerBuilder) buildHTTPConnectionManager(httpOpts *httpListenerOpts connectionManager.HttpFilters = filters connectionManager.RequestIdExtension = requestidextension.BuildUUIDRequestIDExtension(reqIDExtensionCtx) - if features.EnableHCMInternalNetworks && lb.push.Networks != nil { + // If UseRemoteAddress is set, we must set the internal address config in preparation for envoy + // internal addresses defaulting to empty set. Currently, the internal addresses defaulted to + // all private IPs but this will change in the future. + if (features.EnableHCMInternalNetworks || httpOpts.useRemoteAddress) && lb.push.Networks != nil { for _, internalnetwork := range lb.push.Networks.Networks { iac := &hcm.HttpConnectionManager_InternalAddressConfig{} for _, ne := range internalnetwork.Endpoints { diff --git a/pilot/pkg/networking/core/listener_builder_test.go b/pilot/pkg/networking/core/listener_builder_test.go index fccea2b0b843..0aafc7267864 100644 --- a/pilot/pkg/networking/core/listener_builder_test.go +++ b/pilot/pkg/networking/core/listener_builder_test.go @@ -887,6 +887,75 @@ func TestHCMInternalAddressConfig(t *testing.T) { } } +func TestUseRemoteAddressInternalAddressConfig(t *testing.T) { + cg := NewConfigGenTest(t, TestOptions{}) + sidecarProxy := cg.SetupProxy(&model.Proxy{ConfigNamespace: "not-default"}) + push := cg.PushContext() + cases := []struct { + name string + networks *meshconfig.MeshNetworks + expectedconfig *hcm.HttpConnectionManager_InternalAddressConfig + }{ + { + name: "nil networks", + expectedconfig: nil, + }, + { + name: "empty networks", + networks: &meshconfig.MeshNetworks{}, + expectedconfig: nil, + }, + { + name: "networks populated", + networks: &meshconfig.MeshNetworks{ + Networks: map[string]*meshconfig.Network{ + "default": { + Endpoints: []*meshconfig.Network_NetworkEndpoints{ + { + Ne: &meshconfig.Network_NetworkEndpoints_FromCidr{ + FromCidr: "192.168.0.0/16", + }, + }, + { + Ne: &meshconfig.Network_NetworkEndpoints_FromCidr{ + FromCidr: "172.16.0.0/12", + }, + }, + }, + }, + }, + }, + expectedconfig: &hcm.HttpConnectionManager_InternalAddressConfig{ + CidrRanges: []*core.CidrRange{ + { + AddressPrefix: "192.168.0.0", + PrefixLen: &wrapperspb.UInt32Value{Value: 16}, + }, + { + AddressPrefix: "172.16.0.0", + PrefixLen: &wrapperspb.UInt32Value{Value: 12}, + }, + }, + }, + }, + } + for _, tt := range cases { + t.Run(tt.name, func(t *testing.T) { + push.Networks = tt.networks + lb := &ListenerBuilder{ + push: push, + node: sidecarProxy, + authzCustomBuilder: &authz.Builder{}, + authzBuilder: &authz.Builder{}, + } + httpConnManager := lb.buildHTTPConnectionManager(&httpListenerOpts{useRemoteAddress: true}) + if !reflect.DeepEqual(tt.expectedconfig, httpConnManager.InternalAddressConfig) { + t.Errorf("unexpected internal address config, expected: %v, got :%v", tt.expectedconfig, httpConnManager.InternalAddressConfig) + } + }) + } +} + func TestAdditionalAddressesForIPv6(t *testing.T) { test.SetForTest(t, &features.EnableAdditionalIpv4OutboundListenerForIpv6Only, true) cg := NewConfigGenTest(t, TestOptions{Services: testServices}) diff --git a/pkg/bootstrap/instance_test.go b/pkg/bootstrap/instance_test.go index 7cf20c70536c..1068fa5814a2 100644 --- a/pkg/bootstrap/instance_test.go +++ b/pkg/bootstrap/instance_test.go @@ -113,6 +113,9 @@ func TestGolden(t *testing.T) { { base: "default", }, + { + base: "explicit_internal_address", + }, { base: "running", envVars: map[string]string{ diff --git a/pkg/bootstrap/testdata/explicit_internal_address.proxycfg b/pkg/bootstrap/testdata/explicit_internal_address.proxycfg new file mode 100644 index 000000000000..f8f7a21a59c4 --- /dev/null +++ b/pkg/bootstrap/testdata/explicit_internal_address.proxycfg @@ -0,0 +1,12 @@ +config_path: "/etc/istio/proxy" +binary_path: "/usr/local/bin/envoy" +service_cluster: "istio-proxy" +drain_duration: {seconds: 2} +discovery_address: "istio-pilot:15010" +proxy_admin_port: 15000 +control_plane_auth_policy: NONE +runtime_values: [{ key: "envoy.reloadable_features.explicit_internal_address_config" value: "true" }] + +# +# This matches the default configuration hardcoded in model.DefaultProxyConfig +# Flags may override this configuration, as specified by the injector configs. diff --git a/pkg/bootstrap/testdata/explicit_internal_address_golden.json b/pkg/bootstrap/testdata/explicit_internal_address_golden.json new file mode 100644 index 000000000000..e315bfcc91e2 --- /dev/null +++ b/pkg/bootstrap/testdata/explicit_internal_address_golden.json @@ -0,0 +1,429 @@ +{ + "application_log_config": { + "log_format": { + "text_format": "%Y-%m-%dT%T.%fZ\t%l\tenvoy %n %g:%#\t%v\tthread=%t" + } + }, + "node": { + "id": "sidecar~1.2.3.4~foo~bar", + "cluster": "istio-proxy", + "locality": { + }, + "metadata": {"ENVOY_PROMETHEUS_PORT":15090,"ENVOY_STATUS_PORT":15021,"INSTANCE_IPS":"10.3.3.3,10.4.4.4,10.5.5.5,10.6.6.6","ISTIO_VERSION":"binary-1.0","OUTLIER_LOG_PATH":"/dev/stdout","PILOT_SAN":["spiffe://cluster.local/ns/istio-system/sa/istio-pilot-service-account"],"PROXY_CONFIG":{"binaryPath":"/usr/local/bin/envoy","configPath":"/tmp/bootstrap/explicit_internal_address","customConfigFile":"envoy_bootstrap.json","discoveryAddress":"istio-pilot:15010","drainDuration":"2s","proxyAdminPort":15000,"runtimeValues":{"envoy.reloadable_features.explicit_internal_address_config":"true"},"serviceCluster":"istio-proxy","statusPort":15020}} + }, + "layered_runtime": { + "layers": [ + { + "name": "global config", + "static_layer": {"envoy.deprecated_features:envoy.config.listener.v3.Listener.hidden_envoy_deprecated_use_original_dst":true,"envoy.reloadable_features.explicit_internal_address_config":true,"envoy.reloadable_features.http_reject_path_with_fragment":false,"overload.global_downstream_max_connections":"2147483647","re2.max_program_size.error_level":"32768"} + }, + { + "name": "admin", + "admin_layer": {} + } + ] + }, + "bootstrap_extensions": [ + { + "name": "envoy.bootstrap.internal_listener", + "typed_config": { + "@type":"type.googleapis.com/udpa.type.v1.TypedStruct", + "type_url": "type.googleapis.com/envoy.extensions.bootstrap.internal_listener.v3.InternalListener", + "value": { + "buffer_size_kb": 64 + } + } + } + ], + "stats_config": { + "use_all_default_tags": false, + "stats_tags": [ + { + "tag_name": "cluster_name", + "regex": "^cluster\\.((.+?(\\..+?\\.svc\\.cluster\\.local)?)\\.)" + }, + { + "tag_name": "tcp_prefix", + "regex": "^tcp\\.((.*?)\\.)\\w+?$" + }, + { + "regex": "_rq(_(\\d{3}))$", + "tag_name": "response_code" + }, + { + "tag_name": "response_code_class", + "regex": "_rq(_(\\dxx))$" + }, + { + "tag_name": "http_conn_manager_listener_prefix", + "regex": "^listener(?=\\.).*?\\.http\\.(((?:[_.[:digit:]]*|[_\\[\\]aAbBcCdDeEfF[:digit:]]*))\\.)" + }, + { + "tag_name": "http_conn_manager_prefix", + "regex": "^http\\.(((?:[_.[:digit:]]*|[_\\[\\]aAbBcCdDeEfF[:digit:]]*))\\.)" + }, + { + "tag_name": "listener_address", + "regex": "^listener\\.(((?:[_.[:digit:]]*|[_\\[\\]aAbBcCdDeEfF[:digit:]]*))\\.)" + }, + { + "tag_name": "mongo_prefix", + "regex": "^mongo\\.(.+?)\\.(collection|cmd|cx_|op_|delays_|decoding_)(.*?)$" + }, + { + "regex": "(cache\\.(.+?)\\.)", + "tag_name": "cache" + }, + { + "regex": "(component\\.(.+?)\\.)", + "tag_name": "component" + }, + { + "regex": "(tag\\.(.+?);\\.)", + "tag_name": "tag" + }, + { + "regex": "(wasm_filter\\.(.+?)\\.)", + "tag_name": "wasm_filter" + }, + { + "tag_name": "authz_enforce_result", + "regex": "rbac(\\.(allowed|denied))" + }, + { + "tag_name": "authz_dry_run_action", + "regex": "(\\.istio_dry_run_(allow|deny)_)" + }, + { + "tag_name": "authz_dry_run_result", + "regex": "(\\.shadow_(allowed|denied))" + } + ], + "stats_matcher": { + "inclusion_list": { + "patterns": [ + { + "prefix": "reporter=" + }, + { + "prefix": "cluster_manager" + }, + { + "prefix": "listener_manager" + }, + { + "prefix": "server" + }, + { + "prefix": "cluster.xds-grpc" + }, + { + "prefix": "wasm" + }, + { + "suffix": "rbac.allowed" + }, + { + "suffix": "rbac.denied" + }, + { + "suffix": "shadow_allowed" + }, + { + "suffix": "shadow_denied" + }, + { + "safe_regex": {"regex":"vhost\\.*\\.route\\.*"} + }, + { + "prefix": "component" + }, + { + "prefix": "istio" + } + ] + } + } + }, + "admin": { + "access_log": [ + { + "name": "envoy.access_loggers.file", + "typed_config": { + "@type": "type.googleapis.com/envoy.extensions.access_loggers.file.v3.FileAccessLog", + "path": "/dev/null" + } + } + ], + "profile_path": "/var/lib/istio/data/envoy.prof", + "address": { + "socket_address": { + "address": "127.0.0.1", + "port_value": 15000 + } + } + }, + "dynamic_resources": { + "lds_config": { + "ads": {}, + "initial_fetch_timeout": "0s", + "resource_api_version": "V3" + }, + "cds_config": { + "ads": {}, + "initial_fetch_timeout": "0s", + "resource_api_version": "V3" + }, + "ads_config": { + "api_type": "DELTA_GRPC", + "set_node_on_first_message_only": true, + "transport_api_version": "V3", + "grpc_services": [ + { + "envoy_grpc": { + "cluster_name": "xds-grpc" + } + } + ] + } + }, + "static_resources": { + "clusters": [ + { + "name": "prometheus_stats", + "type": "STATIC", + "connect_timeout": "0.250s", + "lb_policy": "ROUND_ROBIN", + "load_assignment": { + "cluster_name": "prometheus_stats", + "endpoints": [{ + "lb_endpoints": [{ + "endpoint": { + "address":{ + "socket_address": { + "protocol": "TCP", + "address": "127.0.0.1", + "port_value": 15000 + } + } + } + }] + }] + } + }, + { + "name": "agent", + "type": "STATIC", + "connect_timeout": "0.250s", + "lb_policy": "ROUND_ROBIN", + "load_assignment": { + "cluster_name": "agent", + "endpoints": [{ + "lb_endpoints": [{ + "endpoint": { + "address":{ + "socket_address": { + "protocol": "TCP", + "address": "127.0.0.1", + "port_value": 15020 + } + } + } + }] + }] + } + }, + { + "name": "sds-grpc", + "type": "STATIC", + "typed_extension_protocol_options": { + "envoy.extensions.upstreams.http.v3.HttpProtocolOptions": { + "@type": "type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions", + "explicit_http_config": { + "http2_protocol_options": {} + } + } + }, + "connect_timeout": "1s", + "lb_policy": "ROUND_ROBIN", + "load_assignment": { + "cluster_name": "sds-grpc", + "endpoints": [{ + "lb_endpoints": [{ + "endpoint": { + "address":{ + "pipe": { + "path": "./var/run/secrets/workload-spiffe-uds/socket" + } + } + } + }] + }] + } + }, + { + "name": "xds-grpc", + "type" : "STATIC", + "connect_timeout": "1s", + "lb_policy": "ROUND_ROBIN", + "load_assignment": { + "cluster_name": "xds-grpc", + "endpoints": [{ + "lb_endpoints": [{ + "endpoint": { + "address":{ + "pipe": { + "path": "/tmp/XDS" + } + } + } + }] + }] + }, + "circuit_breakers": { + "thresholds": [ + { + "priority": "DEFAULT", + "max_connections": 100000, + "max_pending_requests": 100000, + "max_requests": 100000 + }, + { + "priority": "HIGH", + "max_connections": 100000, + "max_pending_requests": 100000, + "max_requests": 100000 + } + ] + }, + "upstream_connection_options": { + "tcp_keepalive": { + "keepalive_time": 300 + } + }, + "max_requests_per_connection": 1, + "typed_extension_protocol_options": { + "envoy.extensions.upstreams.http.v3.HttpProtocolOptions": { + "@type": "type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions", + "explicit_http_config": { + "http2_protocol_options": {} + } + } + } + } + + + ], + "listeners":[ + { + "address": { + "socket_address": { + "protocol": "TCP", + "address": "0.0.0.0", + + "port_value": 15090 + } + }, + "filter_chains": [ + { + "filters": [ + { + "name": "envoy.filters.network.http_connection_manager", + "typed_config": { + "@type": "type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager", + "codec_type": "AUTO", + "stat_prefix": "stats", + "route_config": { + "virtual_hosts": [ + { + "name": "backend", + "domains": [ + "*" + ], + "routes": [ + { + "match": { + "prefix": "/stats/prometheus" + }, + "route": { + "cluster": "prometheus_stats" + } + } + ] + } + ] + }, + "http_filters": [ + { + "name": "envoy.filters.http.router", + "typed_config": { + "@type": "type.googleapis.com/envoy.extensions.filters.http.router.v3.Router" + } + }] + } + } + ] + } + ] + }, + { + "address": { + "socket_address": { + "protocol": "TCP", + "address": "0.0.0.0", + "port_value": 15021 + } + }, + "filter_chains": [ + { + "filters": [ + { + "name": "envoy.filters.network.http_connection_manager", + "typed_config": { + "@type": "type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager", + "codec_type": "AUTO", + "stat_prefix": "agent", + "route_config": { + "virtual_hosts": [ + { + "name": "backend", + "domains": [ + "*" + ], + "routes": [ + { + "match": { + "prefix": "/healthz/ready" + }, + "route": { + "cluster": "agent" + } + } + ] + } + ] + }, + "http_filters": [{ + "name": "envoy.filters.http.router", + "typed_config": { + "@type": "type.googleapis.com/envoy.extensions.filters.http.router.v3.Router" + } + }] + } + } + ] + } + ] + } + ] + } + + + , + "cluster_manager": { + "outlier_detection": { + "event_log_path": "/dev/stdout" + } + } + +}