diff --git a/adapter/kubernetes/config/config.proto b/adapter/kubernetes/config/config.proto index 89655337a..7ad6d59c7 100644 --- a/adapter/kubernetes/config/config.proto +++ b/adapter/kubernetes/config/config.proto @@ -41,7 +41,7 @@ option (gogoproto.gostring_all) = false; // that includes a key of "sourcePodIP" (assuming parameter defaults). message Params { reserved 17; - // next field id: 24 + // next field id: 26 // File path to discover kubeconfig. For in-cluster configuration, // this should be left unset. For local configuration, this should @@ -167,4 +167,17 @@ message Params { // // Default: Service string service_value_name = 16; + + // Whether or not to execute source and origin value lookup + // for Mixer requests when Istio ingress is the destination + // service. + // + // Default: false + bool lookup_ingress_source_and_origin_values = 24; + + // Istio ingress service string. This is used to identify the + // ingress service in requests. + // + // Default: "ingress.istio-system.svc.cluster.local" + string fully_qualified_istio_ingress_service_name = 25; } diff --git a/adapter/kubernetes/kubernetes.go b/adapter/kubernetes/kubernetes.go index 2a38d0ee9..4617a6a4f 100644 --- a/adapter/kubernetes/kubernetes.go +++ b/adapter/kubernetes/kubernetes.go @@ -86,9 +86,11 @@ const ( serviceVal = "Service" // value extraction - clusterDomain = "svc.cluster.local" - podServiceLabel = "app" - istioPodServiceLabel = "istio" + clusterDomain = "svc.cluster.local" + podServiceLabel = "app" + istioPodServiceLabel = "istio" + lookupIngressSourceAndOriginValues = false + istioIngressSvc = "ingress.istio-system.svc.cluster.local" // cache invaliation // TODO: determine a reasonable default @@ -97,27 +99,29 @@ const ( var ( conf = &config.Params{ - KubeconfigPath: "", - CacheRefreshDuration: defaultRefreshPeriod, - SourceUidInputName: sourceUID, - DestinationUidInputName: destinationUID, - OriginUidInputName: originUID, - SourceIpInputName: sourceIP, - DestinationIpInputName: destinationIP, - OriginIpInputName: originIP, - ClusterDomainName: clusterDomain, - PodLabelForService: podServiceLabel, - PodLabelForIstioComponentService: istioPodServiceLabel, - SourcePrefix: sourcePrefix, - DestinationPrefix: destinationPrefix, - OriginPrefix: originPrefix, - LabelsValueName: labelsVal, - PodNameValueName: podNameVal, - PodIpValueName: podIPVal, - HostIpValueName: hostIPVal, - NamespaceValueName: namespaceVal, - ServiceAccountValueName: serviceAccountVal, - ServiceValueName: serviceVal, + KubeconfigPath: "", + CacheRefreshDuration: defaultRefreshPeriod, + SourceUidInputName: sourceUID, + DestinationUidInputName: destinationUID, + OriginUidInputName: originUID, + SourceIpInputName: sourceIP, + DestinationIpInputName: destinationIP, + OriginIpInputName: originIP, + ClusterDomainName: clusterDomain, + PodLabelForService: podServiceLabel, + PodLabelForIstioComponentService: istioPodServiceLabel, + SourcePrefix: sourcePrefix, + DestinationPrefix: destinationPrefix, + OriginPrefix: originPrefix, + LabelsValueName: labelsVal, + PodNameValueName: podNameVal, + PodIpValueName: podIPVal, + HostIpValueName: hostIPVal, + NamespaceValueName: namespaceVal, + ServiceAccountValueName: serviceAccountVal, + ServiceValueName: serviceVal, + FullyQualifiedIstioIngressServiceName: istioIngressSvc, + LookupIngressSourceAndOriginValues: lookupIngressSourceAndOriginValues, } ) @@ -199,6 +203,9 @@ func (*builder) ValidateConfig(c adapter.Config) (ce *adapter.ConfigErrors) { if len(params.PodLabelForIstioComponentService) == 0 { ce = ce.Appendf("podLabelForIstioComponentService", "field must be populated") } + if len(params.FullyQualifiedIstioIngressServiceName) == 0 { + ce = ce.Appendf("fullyQualifiedIstioIngressServiceName", "field must be populated") + } if len(params.ClusterDomainName) == 0 { ce = ce.Appendf("clusterDomainName", "field must be populated") } else if len(strings.Split(params.ClusterDomainName, ".")) != 3 { @@ -264,12 +271,15 @@ func (k *kubegen) Close() error { return nil } func (k *kubegen) Generate(inputs map[string]interface{}) (map[string]interface{}, error) { values := make(map[string]interface{}) - if id, found := serviceIdentifier(inputs, k.params.SourceUidInputName, k.params.SourceIpInputName); found && len(id) > 0 { - k.addValues(values, id, k.params.SourcePrefix) - } if id, found := serviceIdentifier(inputs, k.params.DestinationUidInputName, k.params.DestinationIpInputName); found && len(id) > 0 { k.addValues(values, id, k.params.DestinationPrefix) } + if k.skipIngressLookups(values) { + return values, nil + } + if id, found := serviceIdentifier(inputs, k.params.SourceUidInputName, k.params.SourceIpInputName); found && len(id) > 0 { + k.addValues(values, id, k.params.SourcePrefix) + } if id, found := serviceIdentifier(inputs, k.params.OriginUidInputName, k.params.OriginIpInputName); found && len(id) > 0 { k.addValues(values, id, k.params.OriginPrefix) } @@ -286,6 +296,11 @@ func (k *kubegen) addValues(vals map[string]interface{}, uid, valPrefix string) addPodValues(vals, valPrefix, k.params, pod) } +func (k *kubegen) skipIngressLookups(values map[string]interface{}) bool { + destSvcParam := k.params.DestinationPrefix + k.params.ServiceValueName + return !k.params.LookupIngressSourceAndOriginValues && values[destSvcParam] == k.params.FullyQualifiedIstioIngressServiceName +} + func keyFromUID(uid string) string { if ip := net.ParseIP(uid); ip != nil { return uid @@ -314,10 +329,10 @@ func addPodValues(m map[string]interface{}, prefix string, params config.Params, m[valueName(prefix, params.ServiceAccountValueName)] = p.Spec.ServiceAccountName } if len(p.Status.PodIP) > 0 { - m[valueName(prefix, params.PodIpValueName)] = p.Status.PodIP + m[valueName(prefix, params.PodIpValueName)] = net.ParseIP(p.Status.PodIP) } if len(p.Status.HostIP) > 0 { - m[valueName(prefix, params.HostIpValueName)] = p.Status.HostIP + m[valueName(prefix, params.HostIpValueName)] = net.ParseIP(p.Status.HostIP) } if app, found := p.Labels[params.PodLabelForService]; found { n, err := canonicalName(app, p.Namespace, params.ClusterDomainName) diff --git a/adapter/kubernetes/kubernetes_test.go b/adapter/kubernetes/kubernetes_test.go index 2c7b3549a..3a17738df 100644 --- a/adapter/kubernetes/kubernetes_test.go +++ b/adapter/kubernetes/kubernetes_test.go @@ -97,8 +97,11 @@ func TestBuilder(t *testing.T) { func TestBuilder_ValidateConfigErrors(t *testing.T) { val := reflect.ValueOf(&config.Params{}).Elem() - // currently two non-validated fields: kubeconfig and cache refresh duration - expectedConfigErrs := val.NumField() - 2 + // currently three non-validated fields: + // - kubeconfig + // - cache refresh duration + // - lookup_ingress_source_and_origin_values + expectedConfigErrs := val.NumField() - 3 tests := []struct { name string conf *config.Params @@ -228,12 +231,10 @@ func TestKubegen_Generate(t *testing.T) { "testns/alt-pod": {ObjectMeta: metav1.ObjectMeta{Name: "alt-pod", Namespace: "testns", Labels: map[string]string{"app": "alt-svc.testns"}}}, "testns/bad-svc-pod": {ObjectMeta: metav1.ObjectMeta{Name: "bad-svc-pod", Namespace: "testns", Labels: map[string]string{"app": ":"}}}, "192.168.234.3": {ObjectMeta: metav1.ObjectMeta{Name: "ip-svc-pod", Namespace: "testns", Labels: map[string]string{"app": "ipAddr"}}}, - "testns/istio-ingress": {ObjectMeta: metav1.ObjectMeta{Name: "istio-ingress", Namespace: "testns", Labels: map[string]string{"istio": "ingress"}}}, + "istio-system/ingress": {ObjectMeta: metav1.ObjectMeta{Name: "ingress", Namespace: "istio-system", Labels: map[string]string{"istio": "ingress"}}}, "testns/ipApp": {ObjectMeta: metav1.ObjectMeta{Name: "ipApp", Namespace: "testns", Labels: map[string]string{"app": "10.1.10.1"}}}, } - kg := &kubegen{log: test.NewEnv(t).Logger(), params: *conf, pods: fakeCache{pods: pods}} - sourceUIDIn := map[string]interface{}{ "sourceUID": "kubernetes://test-pod.testns", "destinationUID": "kubernetes://badsvcuid", @@ -245,8 +246,8 @@ func TestKubegen_Generate(t *testing.T) { "app": "test", "something": "", }, - "sourcePodIP": "10.10.10.1", - "sourceHostIP": "10.1.1.10", + "sourcePodIP": net.ParseIP("10.10.10.1"), + "sourceHostIP": net.ParseIP("10.1.1.10"), "sourceNamespace": "testns", "sourcePodName": "test-pod", "sourceService": "test.testns.svc.cluster.local", @@ -296,9 +297,9 @@ func TestKubegen_Generate(t *testing.T) { "destinationPodName": "empty", } - baddestinationSvcIn := map[string]interface{}{"destinationUID": "kubernetes://bad-svc-pod.testns"} + badDestinationSvcIn := map[string]interface{}{"destinationUID": "kubernetes://bad-svc-pod.testns"} - baddestinationOut := map[string]interface{}{ + badDestinationOut := map[string]interface{}{ "destinationLabels": map[string]string{ "app": ":", }, @@ -306,9 +307,9 @@ func TestKubegen_Generate(t *testing.T) { "destinationPodName": "bad-svc-pod", } - ipdestinationSvcIn := map[string]interface{}{"destinationIP": []uint8(net.ParseIP("192.168.234.3"))} + ipDestinationSvcIn := map[string]interface{}{"destinationIP": []uint8(net.ParseIP("192.168.234.3"))} - ipdestinationOut := map[string]interface{}{ + ipDestinationOut := map[string]interface{}{ "destinationLabels": map[string]string{ "app": "ipAddr", }, @@ -317,24 +318,39 @@ func TestKubegen_Generate(t *testing.T) { "destinationService": "ipAddr.testns.svc.cluster.local", } - istiodestinationSvcIn := map[string]interface{}{ - "destinationUID": "kubernetes://istio-ingress.testns", + istioDestinationSvcIn := map[string]interface{}{ + "destinationUID": "kubernetes://ingress.istio-system", + "sourceUID": "kubernetes://test-pod.testns", } - istiodestinationOut := map[string]interface{}{ + istioDestinationOut := map[string]interface{}{ "destinationLabels": map[string]string{ "istio": "ingress", }, - "destinationNamespace": "testns", - "destinationPodName": "istio-ingress", - "destinationService": "ingress.testns.svc.cluster.local", + "destinationNamespace": "istio-system", + "destinationPodName": "ingress", + "destinationService": "ingress.istio-system.svc.cluster.local", + } + + istioDestinationWithSrcOut := map[string]interface{}{ + "destinationLabels": map[string]string{"istio": "ingress"}, + "destinationNamespace": "istio-system", + "destinationPodName": "ingress", + "destinationService": "ingress.istio-system.svc.cluster.local", + "sourceServiceAccountName": "test", + "sourceService": "test.testns.svc.cluster.local", + "sourceLabels": map[string]string{"app": "test", "something": ""}, + "sourceNamespace": "testns", + "sourcePodIP": net.ParseIP("10.10.10.1"), + "sourceHostIP": net.ParseIP("10.1.1.10"), + "sourcePodName": "test-pod", } ipAppSvcIn := map[string]interface{}{ "destinationUID": "kubernetes://ipApp.testns", } - ipAppdestinationOut := map[string]interface{}{ + ipAppDestinationOut := map[string]interface{}{ "destinationLabels": map[string]string{ "app": "10.1.10.1", }, @@ -342,24 +358,32 @@ func TestKubegen_Generate(t *testing.T) { "destinationPodName": "ipApp", } + confWithIngressLookups := *conf + confWithIngressLookups.LookupIngressSourceAndOriginValues = true + tests := []struct { name string inputs map[string]interface{} want map[string]interface{} + params config.Params }{ - {"source pod and destination service", sourceUIDIn, sourceUIDOut}, - {"alternate service canonicalization (namespace)", nsAppLabelIn, nsAppLabelOut}, - {"alternate service canonicalization (svc cluster)", svcClusterIn, svcClusterOut}, - {"alternate service canonicalization (long svc)", longSvcClusterIn, longSvcClusterOut}, - {"empty service", emptySvcIn, emptyServiceOut}, - {"bad destination service", baddestinationSvcIn, baddestinationOut}, - {"destination ip pod", ipdestinationSvcIn, ipdestinationOut}, - {"istio service", istiodestinationSvcIn, istiodestinationOut}, - {"ip app", ipAppSvcIn, ipAppdestinationOut}, + {"source pod and destination service", sourceUIDIn, sourceUIDOut, *conf}, + {"alternate service canonicalization (namespace)", nsAppLabelIn, nsAppLabelOut, *conf}, + {"alternate service canonicalization (svc cluster)", svcClusterIn, svcClusterOut, *conf}, + {"alternate service canonicalization (long svc)", longSvcClusterIn, longSvcClusterOut, *conf}, + {"empty service", emptySvcIn, emptyServiceOut, *conf}, + {"bad destination service", badDestinationSvcIn, badDestinationOut, *conf}, + {"destination ip pod", ipDestinationSvcIn, ipDestinationOut, *conf}, + {"istio ingress service (no lookup source)", istioDestinationSvcIn, istioDestinationOut, *conf}, + {"istio ingress service (lookup source)", istioDestinationSvcIn, istioDestinationWithSrcOut, confWithIngressLookups}, + {"ip app", ipAppSvcIn, ipAppDestinationOut, *conf}, } for _, v := range tests { t.Run(v.name, func(t *testing.T) { + + kg := &kubegen{log: test.NewEnv(t).Logger(), params: v.params, pods: fakeCache{pods: pods}} + got, err := kg.Generate(v.inputs) if err != nil { t.Errorf("Unexpected error: %v", err) diff --git a/pkg/aspect/attrgenmgr.go b/pkg/aspect/attrgenmgr.go index 11172c9d4..893faa417 100644 --- a/pkg/aspect/attrgenmgr.go +++ b/pkg/aspect/attrgenmgr.go @@ -16,6 +16,7 @@ package aspect import ( "fmt" + "net" "github.com/golang/glog" rpc "github.com/googleapis/googleapis/google/rpc" @@ -106,7 +107,19 @@ func (e *attrGenExec) Execute(attrs attribute.Bag, mapper expr.Evaluator) (*Prep for key, val := range out { if attrName, found := e.bindings[key]; found { // TODO: type validation? - bag.Set(attrName, val) + switch v := val.(type) { + case net.IP: + // conversion to []byte necessary based on current IP_ADDRESS handling within Mixer + // TODO: remove + glog.Warningf("---- converting net.IP to []byte") + if v4 := v.To4(); v4 != nil { + bag.Set(attrName, []byte(v4)) + continue + } + bag.Set(attrName, []byte(v.To16())) + default: + bag.Set(attrName, val) + } continue } glog.Warningf("Generated value '%s' was not mapped to an attribute.", key) diff --git a/pkg/aspect/attrgenmgr_test.go b/pkg/aspect/attrgenmgr_test.go index cc2f7aec2..2f1291ef9 100644 --- a/pkg/aspect/attrgenmgr_test.go +++ b/pkg/aspect/attrgenmgr_test.go @@ -16,6 +16,8 @@ package aspect import ( "errors" + "net" + "reflect" "testing" dpb "istio.io/api/mixer/v1/config/descriptor" @@ -217,26 +219,36 @@ func (t testAttrGen) Generate(map[string]interface{}) (map[string]interface{}, e func TestAttributeGeneratorExecutor_Execute(t *testing.T) { genParams := &apb.AttributesGeneratorParams{ - InputExpressions: map[string]string{"pod.ip": "source_ip"}, - AttributeBindings: map[string]string{"service_found": "found", "source_service": "srcSvc"}, + InputExpressions: map[string]string{"pod.ip": "source_ip"}, + AttributeBindings: map[string]string{ + "service_found": "found", + "source_service": "srcSvc", + "destination_ip": "destIP", + "ip_v6": "v6IP", + }, } - bMap := map[string]string{"found": "service_found", "srcSvc": "source_service"} + bMap := map[string]string{"found": "service_found", "srcSvc": "source_service", "destIP": "destination_ip", "v6IP": "ip_v6"} - inBag := attribute.GetMutableBag(nil) - inBag.Set("source_ip", "10.1.1.10") + inBag := attribute.GetFakeMutableBagForTesting(map[string]interface{}{"source_ip": []byte(net.IP("10.1.1.10").To4())}) outMap := map[string]interface{}{ "found": true, "srcSvc": "service1", + "destIP": net.ParseIP("10.34.23.3"), + "v6IP": net.ParseIP("2001:db8::1"), } wantBag := attribute.GetMutableBag(nil) wantBag.Set("service_found", true) wantBag.Set("source_service", "service1") + wantBag.Set("destination_ip", []byte{0xa, 0x22, 0x17, 0x3}) + wantBag.Set("ip_v6", []byte{0x20, 0x1, 0xd, 0xb8, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1}) extraOutMap := map[string]interface{}{ "found": true, "srcSvc": "service1", + "destIP": net.ParseIP("10.34.23.3"), + "v6IP": net.ParseIP("2001:db8::1"), "shouldBeStripped": "never_used", } @@ -272,7 +284,7 @@ func TestAttributeGeneratorExecutor_Execute(t *testing.T) { if !ok { t.Errorf("Generated attribute.Bag missing attribute %s", n) } - if gotVal != wantVal { + if !reflect.DeepEqual(gotVal, wantVal) { t.Errorf("For attribute '%s': got value %v, want %v", n, gotVal, wantVal) } } diff --git a/testdata/configroot/scopes/global/subjects/global/rules.yml b/testdata/configroot/scopes/global/subjects/global/rules.yml index b9799c933..001dee164 100644 --- a/testdata/configroot/scopes/global/subjects/global/rules.yml +++ b/testdata/configroot/scopes/global/subjects/global/rules.yml @@ -14,12 +14,12 @@ rules: destinationUID: destination.uid | "" destinationIP: destination.ip | ip("0.0.0.0") # default to unspecified ip addr attribute_bindings: - source.ip: sourcePodIp + source.ip: sourcePodIP source.labels: sourceLabels source.namespace: sourceNamespace source.service: sourceService source.serviceAccount: sourceServiceAccountName - destination.ip: destinationPodIp + destination.ip: destinationPodIP destination.labels: destinationLabels destination.namespace: destinationNamespace destination.service: destinationService