diff --git a/pilot/pkg/model/service.go b/pilot/pkg/model/service.go index ca26982e6a36..a17ab4808c82 100644 --- a/pilot/pkg/model/service.go +++ b/pilot/pkg/model/service.go @@ -249,6 +249,19 @@ func (instance *ServiceInstance) GetLocality() string { return GetLocalityOrDefault(instance.Endpoint.Labels[LocalityLabel], instance.Endpoint.Locality) } +// DeepCopy creates a copy of ServiceInstance. +func (instance *ServiceInstance) DeepCopy() *ServiceInstance { + return &ServiceInstance{ + Service: instance.Service.DeepCopy(), + Endpoint: instance.Endpoint.DeepCopy(), + ServicePort: &Port{ + Name: instance.ServicePort.Name, + Port: instance.ServicePort.Port, + Protocol: instance.ServicePort.Protocol, + }, + } +} + // GetLocalityOrDefault returns the locality from the supplied label, or falls back to // the supplied default locality if the supplied label is empty. Because Kubernetes // labels don't support `/`, we replace "." with "/" in the supplied label as a workaround. @@ -665,6 +678,11 @@ func (s *Service) DeepCopy() *Service { } } +// DeepCopy creates a clone of IstioEndpoint. +func (ep *IstioEndpoint) DeepCopy() *IstioEndpoint { + return copyInternal(ep).(*IstioEndpoint) +} + func copyInternal(v interface{}) interface{} { copied, err := copystructure.Copy(v) if err != nil { diff --git a/pilot/pkg/networking/core/v1alpha3/cluster.go b/pilot/pkg/networking/core/v1alpha3/cluster.go index c334789dec95..3bd54963ad20 100644 --- a/pilot/pkg/networking/core/v1alpha3/cluster.go +++ b/pilot/pkg/networking/core/v1alpha3/cluster.go @@ -571,7 +571,6 @@ func (configgen *ConfigGeneratorImpl) buildInboundClusters(proxy *model.Proxy, } } else { rule := sidecarScope.Config.Spec.(*networking.Sidecar) - sidecarScopeID := sidecarScope.Config.Name + "." + sidecarScope.Config.Namespace for _, ingressListener := range rule.Ingress { // LDS would have setup the inbound clusters // as inbound|portNumber|portName|Hostname[or]SidecarScopeID @@ -604,29 +603,9 @@ func (configgen *ConfigGeneratorImpl) buildInboundClusters(proxy *model.Proxy, } // Find the service instance that corresponds to this ingress listener by looking - // for a service instance that either matches this ingress port as this will allow us + // for a service instance that matches this ingress port as this will allow us // to generate the right cluster name that LDS expects inbound|portNumber|portName|Hostname - instance := configgen.findServiceInstanceForIngressListener(instances, ingressListener) - - if instance == nil { - // We didn't find a matching instance. Create a dummy one because we need the right - // params to generate the right cluster name. LDS would have setup the cluster as - // as inbound|portNumber|portName|SidecarScopeID - attrs := model.ServiceAttributes{ - Name: sidecarScope.Config.Name, - Namespace: sidecarScope.Config.Namespace, - } - instance = &model.ServiceInstance{ - Service: &model.Service{ - Hostname: host.Name(sidecarScopeID), - Attributes: attrs, - }, - Endpoint: &model.IstioEndpoint{ - Attributes: attrs, - }, - } - } - + instance := configgen.findOrCreateServiceInstance(instances, ingressListener, sidecarScope.Config.Name, sidecarScope.Config.Namespace) instance.Endpoint.Family = endpointFamily instance.Endpoint.Address = endpointAddress instance.ServicePort = listenPort @@ -648,17 +627,30 @@ func (configgen *ConfigGeneratorImpl) buildInboundClusters(proxy *model.Proxy, return clusters } -func (configgen *ConfigGeneratorImpl) findServiceInstanceForIngressListener(instances []*model.ServiceInstance, - ingressListener *networking.IstioIngressListener) *model.ServiceInstance { - // Search by port +func (configgen *ConfigGeneratorImpl) findOrCreateServiceInstance(instances []*model.ServiceInstance, + ingressListener *networking.IstioIngressListener, sidecar string, sidecarns string) *model.ServiceInstance { for _, realInstance := range instances { if realInstance.Endpoint.EndpointPort == ingressListener.Port.Number { - instanceCopy := *realInstance - return &instanceCopy + // We need to create a copy of the instance, as it is modified later while building clusters/listeners. + return realInstance.DeepCopy() } } - - return nil + // We didn't find a matching instance. Create a dummy one because we need the right + // params to generate the right cluster name i.e. inbound|portNumber|portName|SidecarScopeID - which is uniformly generated by LDS/CDS. + attrs := model.ServiceAttributes{ + Name: sidecar, + // This will ensure that the right AuthN policies are selected + Namespace: sidecarns, + } + return &model.ServiceInstance{ + Service: &model.Service{ + Hostname: host.Name(sidecar + "." + sidecarns), + Attributes: attrs, + }, + Endpoint: &model.IstioEndpoint{ + Attributes: attrs, + }, + } } func (configgen *ConfigGeneratorImpl) buildInboundClusterForPortOrUDS(pluginParams *plugin.InputParams) *apiv2.Cluster { diff --git a/pilot/pkg/networking/core/v1alpha3/cluster_test.go b/pilot/pkg/networking/core/v1alpha3/cluster_test.go index 65593eadffc1..664fb2711afb 100644 --- a/pilot/pkg/networking/core/v1alpha3/cluster_test.go +++ b/pilot/pkg/networking/core/v1alpha3/cluster_test.go @@ -32,6 +32,7 @@ import ( authn "istio.io/api/authentication/v1alpha1" meshconfig "istio.io/api/mesh/v1alpha1" + "istio.io/api/networking/v1alpha3" networking "istio.io/api/networking/v1alpha3" "istio.io/istio/pilot/pkg/features" @@ -1338,6 +1339,55 @@ func TestBuildLocalityLbEndpoints(t *testing.T) { } } +func TestFindServiceInstanceForIngressListener(t *testing.T) { + servicePort := &model.Port{ + Name: "default", + Port: 7443, + Protocol: protocol.HTTP, + } + service := &model.Service{ + Hostname: host.Name("*.example.org"), + Address: "1.1.1.1", + ClusterVIPs: make(map[string]string), + Ports: model.PortList{servicePort}, + Resolution: model.ClientSideLB, + } + + instances := []*model.ServiceInstance{ + { + Service: service, + ServicePort: servicePort, + Endpoint: &model.IstioEndpoint{ + Address: "192.168.1.1", + EndpointPort: 7443, + Locality: "region1/zone1/subzone1", + LbWeight: 30, + }, + }, + } + + ingress := &networking.IstioIngressListener{ + CaptureMode: v1alpha3.CaptureMode_NONE, + DefaultEndpoint: "127.0.0.1:7020", + Port: &v1alpha3.Port{ + Number: 7443, + Name: "grpc-core", + Protocol: "GRPC", + }, + } + configgen := NewConfigGenerator([]plugin.Plugin{}) + instance := configgen.findOrCreateServiceInstance(instances, ingress, "sidecar", "sidecarns") + if instance == nil || instance.Service.Hostname.Matches("sidecar.sidecarns") { + t.Fatal("Expected to return a valid instance, but got nil/default instance") + } + if instance == instances[0] { + t.Fatal("Expected to return a copy of instance, but got the same instance") + } + if !reflect.DeepEqual(instance, instances[0]) { + t.Fatal("Expected returned copy of instance to be equal, but they are different") + } +} + func TestClusterDiscoveryTypeAndLbPolicyRoundRobin(t *testing.T) { g := NewGomegaWithT(t) diff --git a/pilot/pkg/networking/core/v1alpha3/listener.go b/pilot/pkg/networking/core/v1alpha3/listener.go index 7693ae6a75e9..cb2c2e71e0d9 100644 --- a/pilot/pkg/networking/core/v1alpha3/listener.go +++ b/pilot/pkg/networking/core/v1alpha3/listener.go @@ -491,7 +491,6 @@ func (configgen *ConfigGeneratorImpl) buildSidecarInboundListeners( } else { rule := sidecarScope.Config.Spec.(*networking.Sidecar) - sidecarScopeID := sidecarScope.Config.Name + "." + sidecarScope.Config.Namespace for _, ingressListener := range rule.Ingress { // determine the bindToPort setting for listeners. Validation guarantees that these are all IP listeners. bindToPort := false @@ -520,27 +519,8 @@ func (configgen *ConfigGeneratorImpl) buildSidecarInboundListeners( bind = getSidecarInboundBindIP(node) } - instance := configgen.findServiceInstanceForIngressListener(node.ServiceInstances, ingressListener) - - if instance == nil { - // We didn't find a matching instance. Create a dummy one because we need the right - // params to generate the right cluster name. CDS would have setup the cluster as - // as inbound|portNumber|portName|SidecarScopeID - attrs := model.ServiceAttributes{ - Name: sidecarScope.Config.Name, - // This will ensure that the right AuthN policies are selected - Namespace: sidecarScope.Config.Namespace, - } - instance = &model.ServiceInstance{ - Service: &model.Service{ - Hostname: host.Name(sidecarScopeID), - Attributes: attrs, - }, - Endpoint: &model.IstioEndpoint{ - Attributes: attrs, - }, - } - } + instance := configgen.findOrCreateServiceInstance(node.ServiceInstances, ingressListener, + sidecarScope.Config.Name, sidecarScope.Config.Namespace) listenerOpts := buildListenerOpts{ push: push,