diff --git a/mixer/adapter/circonus/operatorconfig/attributes.yaml b/mixer/adapter/circonus/operatorconfig/attributes.yaml index 42b2d03a5817..38cbef0e6b42 100644 --- a/mixer/adapter/circonus/operatorconfig/attributes.yaml +++ b/mixer/adapter/circonus/operatorconfig/attributes.yaml @@ -47,8 +47,6 @@ spec: valueType: STRING source.user: valueType: STRING - target.uid: - valueType: STRING destination.uid: valueType: STRING connection.id: @@ -89,17 +87,6 @@ spec: valueType: STRING source.serviceAccount: valueType: STRING - target.ip: - valueType: IP_ADDRESS - target.labels: - valueType: STRING_MAP - target.name: - valueType: STRING - target.namespace: - valueType: STRING - target.service: - valueType: STRING - target.serviceAccount: valueType: STRING destination.ip: valueType: IP_ADDRESS diff --git a/mixer/adapter/opa/README.md b/mixer/adapter/opa/README.md index 6ba98b1a1d7f..6c75b0927814 100644 --- a/mixer/adapter/opa/README.md +++ b/mixer/adapter/opa/README.md @@ -68,8 +68,8 @@ spec: subject: user: source.uid | "" action: - namespace: target.namespace | "default" - service: target.service | "" + namespace: destination.namespace | "default" + service: destination.service | "" method: request.method | "" path: request.path | "" diff --git a/mixer/adapter/opa/opa_integration_test.go b/mixer/adapter/opa/opa_integration_test.go index cb1afea4fb26..3b7b78360009 100644 --- a/mixer/adapter/opa/opa_integration_test.go +++ b/mixer/adapter/opa/opa_integration_test.go @@ -41,9 +41,9 @@ spec: value_type: STRING source.groups: value_type: STRING - target.namespace: + destination.namespace: value_type: STRING - target.service: + destination.service: value_type: STRING request.method: value_type: STRING @@ -51,7 +51,7 @@ spec: value_type: STRING source.service: value_type: STRING - target.service: + destination.service: value_type: STRING --- @@ -81,13 +81,13 @@ spec: user: source.uid | "" groups: source.groups | "" action: - namespace: target.namespace | "default" - service: target.service | "" + namespace: destination.namespace | "default" + service: destination.service | "" method: request.method | "" path: request.path | "" properties: source: source.service | "" - target: target.service | "" + target: destination.service | "" --- apiVersion: "config.istio.io/v1alpha2" @@ -210,50 +210,45 @@ func TestServer(t *testing.T) { }{ "Not important API": { attrs: map[string]interface{}{ - "destination.service": "svc.cluster.local", "source.uid": "janet", "request.path": "/detail/alice", - "target.service": "landing_page", + "destination.service": "landing_page", "source.service": "details", }, expectedStatusCode: 0, }, "Self permission": { attrs: map[string]interface{}{ - "destination.service": "svc.cluster.local", "source.uid": "janet", "request.path": "/detail/janet", - "target.service": "landing_page", + "destination.service": "landing_page", "source.service": "details", }, expectedStatusCode: 0, }, "Manager permission": { attrs: map[string]interface{}{ - "destination.service": "svc.cluster.local", "source.uid": "janet", "request.path": "/reviews/alice", - "target.service": "landing_page", + "destination.service": "landing_page", "source.service": "details", }, expectedStatusCode: 0, }, "HR permission": { attrs: map[string]interface{}{ - "destination.service": "svc.cluster.local", "source.uid": "ken", "request.path": "/reviews/janet", - "target.service": "landing_page", + "destination.service": "landing_page", "source.service": "details", }, expectedStatusCode: 0, }, "Denied request": { attrs: map[string]interface{}{ - "destination.service": "svc.cluster.local", "source.uid": "janet", "request.path": "/detail/ken", - "target.service": "landing_pages", + "destination.service": "landing_pages", "source.service": "invalid", }, expectedStatusCode: 7, diff --git a/mixer/doc/adapter-development-walkthrough.md b/mixer/doc/adapter-development-walkthrough.md index 94c5bad6b942..86928ac361d4 100644 --- a/mixer/doc/adapter-development-walkthrough.md +++ b/mixer/doc/adapter-development-walkthrough.md @@ -455,10 +455,10 @@ spec: value: "1" dimensions: source: source.labels["app"] | "unknown" - target: target.service | "unknown" - service: target.labels["app"] | "unknown" + target: destination.service | "unknown" + service: destination.labels["app"] | "unknown" method: request.path | "unknown" - version: target.labels["version"] | "unknown" + version: destination.labels["version"] | "unknown" response_code: response.code | 200 monitored_resource_type: '"UNSPECIFIED"' --- @@ -552,7 +552,7 @@ You can even try passing other attributes to mixer server and inspect your out.t the adapter changes. For example ```bash -pushd $MIXER_REPO && go install ./... && mixc report -s="destination.service=svc.cluster.local,target.service=mySrvc" -i="response.code=400" --stringmap_attributes="target.labels=app:dummyapp" +pushd $MIXER_REPO && go install ./... && mixc report -s="destination.service=svc.cluster.local,destination.service=mySrvc" -i="response.code=400" --stringmap_attributes="destination.labels=app:dummyapp" ``` **If you have reached this far, congratulate yourself !!**. You have successfully created a Mixer adapter. You can diff --git a/mixer/doc/running-local-mixer.md b/mixer/doc/running-local-mixer.md index 82384f88b3b7..a9ad088dbc0c 100644 --- a/mixer/doc/running-local-mixer.md +++ b/mixer/doc/running-local-mixer.md @@ -19,7 +19,7 @@ Note that `source.ip` is an ip address specified as 4 `:` separated bytes. `192.0.0.2` is encoded as `c0:0:0:2` in the example. ```shell -bazel-bin/mixer/cmd/mixc/mixc check --string_attributes destination.service=abc.ns.svc.cluster.local,source.name=myservice,target.port=8080 --stringmap_attributes "request.headers=clnt:abcd;source:abcd,destination.labels=app:ratings,source.labels=version:v2" --timestamp_attributes request.time="2017-07-04T00:01:10Z" --bytes_attributes source.ip=c0:0:0:2 +bazel-bin/mixer/cmd/mixc/mixc check --string_attributes destination.service=abc.ns.svc.cluster.local,source.name=myservice,destination.port=8080 --stringmap_attributes "request.headers=clnt:abcd;source:abcd,destination.labels=app:ratings,source.labels=version:v2" --timestamp_attributes request.time="2017-07-04T00:01:10Z" --bytes_attributes source.ip=c0:0:0:2 Check RPC completed successfully. Check status was OK Valid use count: 10000, valid duration: 5m0s @@ -27,7 +27,7 @@ Check RPC completed successfully. Check status was OK The following command sends a `report` request to Mixer. ```shell -bazel-bin/mixer/cmd/mixc/mixc report --string_attributes destination.service=abc.ns.svc.cluster.local,source.name=myservice,target.port=8080 --stringmap_attributes "request.headers=clnt:abc;source:abcd,destination.labels=app:ratings,source.labels=version:v2" --int64_attributes response.duration=2003,response.size=1024 --timestamp_attributes request.time="2017-07-04T00:01:10Z" --bytes_attributes source.ip=c0:0:0:2 +bazel-bin/mixer/cmd/mixc/mixc report --string_attributes destination.service=abc.ns.svc.cluster.local,source.name=myservice,destination.port=8080 --stringmap_attributes "request.headers=clnt:abc;source:abcd,destination.labels=app:ratings,source.labels=version:v2" --int64_attributes response.duration=2003,response.size=1024 --timestamp_attributes request.time="2017-07-04T00:01:10Z" --bytes_attributes source.ip=c0:0:0:2 Report RPC returned OK ``` diff --git a/mixer/pkg/api/grpcServer.go b/mixer/pkg/api/grpcServer.go index 4cbc52ce90e9..d19d03b9f6f6 100644 --- a/mixer/pkg/api/grpcServer.go +++ b/mixer/pkg/api/grpcServer.go @@ -16,7 +16,6 @@ package api import ( "fmt" - "strings" "time" opentracing "github.com/opentracing/opentracing-go" @@ -80,40 +79,6 @@ func NewGRPCServer(dispatcher runtime.Dispatcher, gp *pool.GoroutinePool) mixerp } } -// compatBag implements compatibility between destination.* and target.* attributes. -type compatBag struct { - parent attribute.Bag -} - -func (c *compatBag) String() string { - return c.parent.String() -} - -// if a destination.* attribute is missing, check the corresponding target.* attribute. -func (c *compatBag) Get(name string) (v interface{}, found bool) { - v, found = c.parent.Get(name) - if found { - return - } - if !strings.HasPrefix(name, "destination.") { - return - } - compatAttr := strings.Replace(name, "destination.", "target.", 1) - v, found = c.parent.Get(compatAttr) - if found { - log.Warna("Deprecated attribute found: ", compatAttr) - } - return -} - -func (c *compatBag) Names() []string { - return c.parent.Names() -} - -func (c *compatBag) Done() { - c.parent.Done() -} - // Check is the entry point for the external Check method func (s *grpcServer) Check(legacyCtx legacyContext.Context, req *mixerpb.CheckRequest) (*mixerpb.CheckResponse, error) { // TODO: this code doesn't distinguish between RPC failures when communicating with adapters and @@ -122,42 +87,35 @@ func (s *grpcServer) Check(legacyCtx legacyContext.Context, req *mixerpb.CheckRe // request was denied? This will need to be addressed in the new adapter model. In the meantime, // RPC failure is treated as a semantic denial. - requestBag := attribute.NewProtoBag(&req.Attributes, s.globalDict, s.globalWordList) + log.Debug("Dispatching Preprocess Check") globalWordCount := int(req.GlobalWordCount) - // compatReqBag ensures that preprocessor input handles deprecated attributes gracefully. - compatReqBag := &compatBag{requestBag} - preprocResponseBag := attribute.GetMutableBag(nil) - - log.Debuga("Dispatching Preprocess Check") + protoBag := attribute.NewProtoBag(&req.Attributes, s.globalDict, s.globalWordList) + checkBag := attribute.GetMutableBag(protoBag) - mutableBag := attribute.GetMutableBag(requestBag) var out rpc.Status - if err := s.dispatcher.Preprocess(legacyCtx, compatReqBag, preprocResponseBag); err != nil { + if err := s.dispatcher.Preprocess(legacyCtx, protoBag, checkBag); err != nil { out = status.WithError(err) } - if err := mutableBag.Merge(preprocResponseBag); err != nil { - out = status.WithError(fmt.Errorf("could not merge preprocess attributes into request attributes: %v", err)) - } - - compatRespBag := &compatBag{mutableBag} if !status.IsOK(out) { log.Errora("Preprocess Check returned with: ", status.String(out)) - requestBag.Done() - preprocResponseBag.Done() + protoBag.Done() + checkBag.Done() return nil, makeGRPCError(out) } if log.DebugEnabled() { log.Debuga("Preprocess Check returned with: ", status.String(out)) log.Debug("Dispatching to main adapters after running processors") - log.Debuga("Attribute Bag: \n", mutableBag) + log.Debuga("Attribute Bag: \n", checkBag) log.Debug("Dispatching Check") } - cr, err := s.dispatcher.Check(legacyCtx, compatRespBag) + snap := protoBag.SnapshotReferencedAttributes() + + cr, err := s.dispatcher.Check(legacyCtx, checkBag) if err != nil { out = status.WithError(err) } @@ -174,7 +132,7 @@ func (s *grpcServer) Check(legacyCtx legacyContext.Context, req *mixerpb.CheckRe ValidDuration: cr.ValidDuration, ValidUseCount: cr.ValidUseCount, Status: out, - ReferencedAttributes: requestBag.GetReferencedAttributes(s.globalDict, globalWordCount), + ReferencedAttributes: protoBag.GetReferencedAttributes(s.globalDict, globalWordCount), }, } @@ -183,7 +141,6 @@ func (s *grpcServer) Check(legacyCtx legacyContext.Context, req *mixerpb.CheckRe } else { log.Errora("Check returned with error: ", status.String(out)) } - requestBag.ClearReferencedAttributes() if status.IsOK(resp.Precondition.Status) && len(req.Quotas) > 0 { resp.Quotas = make(map[string]mixerpb.CheckResponse_QuotaResult, len(req.Quotas)) @@ -202,11 +159,13 @@ func (s *grpcServer) Check(legacyCtx legacyContext.Context, req *mixerpb.CheckRe } var err error - qr, err = quota(legacyCtx, s.dispatcher, compatRespBag, qma) + // restore to the post-APA state + protoBag.ResetReferencedAttributes(snap) + + qr, err = quota(legacyCtx, s.dispatcher, checkBag, qma) // if quota check fails, set status for the entire request and stop processing. if err != nil { resp.Precondition.Status = status.WithError(err) - requestBag.ClearReferencedAttributes() break } @@ -219,14 +178,13 @@ func (s *grpcServer) Check(legacyCtx legacyContext.Context, req *mixerpb.CheckRe } } - qr.ReferencedAttributes = requestBag.GetReferencedAttributes(s.globalDict, globalWordCount) + qr.ReferencedAttributes = protoBag.GetReferencedAttributes(s.globalDict, globalWordCount) resp.Quotas[name] = *qr - requestBag.ClearReferencedAttributes() } } - requestBag.Done() - preprocResponseBag.Done() + checkBag.Done() + protoBag.Done() return resp, nil } @@ -273,10 +231,9 @@ func (s *grpcServer) Report(legacyCtx legacyContext.Context, req *mixerpb.Report } protoBag := attribute.NewProtoBag(&req.Attributes[0], s.globalDict, s.globalWordList) - requestBag := attribute.GetMutableBag(protoBag) - compatReqBag := &compatBag{requestBag} - mutableBag := attribute.GetMutableBag(requestBag) - preprocResponseBag := attribute.GetMutableBag(nil) + accumBag := attribute.GetMutableBag(protoBag) + reportBag := attribute.GetMutableBag(accumBag) + var err error for i := 0; i < len(req.Attributes); i++ { span, newctx := opentracing.StartSpanFromContext(legacyCtx, fmt.Sprintf("Attributes %d", i)) @@ -284,7 +241,7 @@ func (s *grpcServer) Report(legacyCtx legacyContext.Context, req *mixerpb.Report // the first attribute block is handled by the protoBag as a foundation, // deltas are applied to the child bag (i.e. requestBag) if i > 0 { - err = requestBag.UpdateBagFromProto(&req.Attributes[i], s.globalWordList) + err = accumBag.UpdateBagFromProto(&req.Attributes[i], s.globalWordList) if err != nil { msg := "Request could not be processed due to invalid attributes." log.Errora(msg, "\n", err) @@ -296,14 +253,10 @@ func (s *grpcServer) Report(legacyCtx legacyContext.Context, req *mixerpb.Report log.Debug("Dispatching Preprocess") var out rpc.Status - if err = s.dispatcher.Preprocess(newctx, compatReqBag, preprocResponseBag); err != nil { + if err = s.dispatcher.Preprocess(newctx, accumBag, reportBag); err != nil { out = status.WithError(err) } - if err = mutableBag.Merge(preprocResponseBag); err != nil { - out = status.WithError(fmt.Errorf("could not merge preprocess attributes into request attributes: %v", err)) - } - compatRespBag := &compatBag{mutableBag} if !status.IsOK(out) { log.Errora("Preprocess returned with: ", status.String(out)) err = makeGRPCError(out) @@ -314,11 +267,12 @@ func (s *grpcServer) Report(legacyCtx legacyContext.Context, req *mixerpb.Report if log.DebugEnabled() { log.Debuga("Preprocess returned with: ", status.String(out)) - log.Debug("Dispatching to main adapters after running processors") - log.Debuga("Attribute Bag: \n", mutableBag) + log.Debug("Dispatching to main adapters after running preprocessors") + log.Debuga("Attribute Bag: \n", reportBag) log.Debugf("Dispatching Report %d out of %d", i, len(req.Attributes)) } - err = s.dispatcher.Report(legacyCtx, compatRespBag) + + err = s.dispatcher.Report(legacyCtx, reportBag) if err != nil { out = status.WithError(err) log.Warnf("Report returned %v", err) @@ -338,11 +292,11 @@ func (s *grpcServer) Report(legacyCtx legacyContext.Context, req *mixerpb.Report span.LogFields(otlog.String("success", "finished Report for attribute bag "+string(i))) span.Finish() - preprocResponseBag.Reset() + reportBag.Reset() } - preprocResponseBag.Done() - requestBag.Done() + reportBag.Done() + accumBag.Done() protoBag.Done() if err != nil { diff --git a/mixer/pkg/api/grpcServer_test.go b/mixer/pkg/api/grpcServer_test.go index 584f49fc1e9c..16ca20d10f61 100644 --- a/mixer/pkg/api/grpcServer_test.go +++ b/mixer/pkg/api/grpcServer_test.go @@ -205,15 +205,11 @@ func TestCheck(t *testing.T) { } ts.preproc = func(ctx context.Context, requestBag attribute.Bag, responseBag *attribute.MutableBag) error { - responseBag.Set("A1", "override") responseBag.Set("genAttrGen", "genAttrGenValue") return nil } ts.check = func(ctx context.Context, requestBag attribute.Bag) (*adapter.CheckResult, error) { - if val, _ := requestBag.Get("A1"); val == "override" { - return nil, errors.New("attribute overriding not allowed in Check") - } if val, _ := requestBag.Get("genAttrGen"); val != "genAttrGenValue" { return nil, errors.New("generated attribute via preproc not part of check attributes") } @@ -371,15 +367,11 @@ func TestReport(t *testing.T) { } ts.preproc = func(ctx context.Context, requestBag attribute.Bag, responseBag *attribute.MutableBag) error { - responseBag.Set("A1", "override") responseBag.Set("genAttrGen", "genAttrGenValue") return nil } ts.report = func(ctx context.Context, requestBag attribute.Bag) error { - if val, _ := requestBag.Get("A1"); val == "override" { - return errors.New("attribute overriding NOT allowed in Report") - } if val, _ := requestBag.Get("genAttrGen"); val != "genAttrGenValue" { return errors.New("generated attribute via preproc not part of report attributes") } diff --git a/mixer/pkg/api/perf_test.go b/mixer/pkg/api/perf_test.go index ceb6bd5fd3db..0508d83a0a47 100644 --- a/mixer/pkg/api/perf_test.go +++ b/mixer/pkg/api/perf_test.go @@ -226,8 +226,8 @@ func getGlobalDict() []string { "response.time", "source.namespace", "source.uid", - "target.namespace", - "target.uid", + "destination.namespace", + "destination.uid", } } @@ -260,6 +260,6 @@ func setRequestAttrs(bag *attribute.MutableBag, uuid []byte) { bag.Set("response.time", time.Now()) bag.Set("source.namespace", "XYZ11") bag.Set("source.uid", "POD11") - bag.Set("target.namespace", "XYZ222") - bag.Set("target.uid", "POD222") + bag.Set("destination.namespace", "XYZ222") + bag.Set("destination.uid", "POD222") } diff --git a/mixer/pkg/attribute/bag_test.go b/mixer/pkg/attribute/bag_test.go index 7c36bf8487e2..3918945a5553 100644 --- a/mixer/pkg/attribute/bag_test.go +++ b/mixer/pkg/attribute/bag_test.go @@ -18,7 +18,6 @@ import ( "fmt" "reflect" "strconv" - "strings" "testing" "time" @@ -130,9 +129,8 @@ func TestMerge(t *testing.T) { c1.Set("STRING1", "A") c2.Set("STRING2", "B") - if err := mb.Merge(c1, nil, c2); err != nil { - t.Errorf("Got %v, expecting success", err) - } + mb.Merge(c1) + mb.Merge(c2) if v, ok := mb.Get("STRING0"); !ok || v.(string) != "@" { t.Errorf("Got %v, expected @", v) @@ -147,22 +145,6 @@ func TestMerge(t *testing.T) { } } -func TestMergeErrors(t *testing.T) { - mb := GetMutableBag(empty) - - c1 := GetMutableBag(mb) - c2 := GetMutableBag(mb) - - c1.Set("FOO", "X") - c2.Set("FOO", "Y") - - if err := mb.Merge(c1, c2); err == nil { - t.Error("Got success, expected failure") - } else if !strings.Contains(err.Error(), "FOO") { - t.Errorf("Expected error to contain the word FOO, got %s", err.Error()) - } -} - func TestEmpty(t *testing.T) { b := &emptyBag{} diff --git a/mixer/pkg/attribute/mutableBag.go b/mixer/pkg/attribute/mutableBag.go index 3b7700d9d35e..f82ddafc8e39 100644 --- a/mixer/pkg/attribute/mutableBag.go +++ b/mixer/pkg/attribute/mutableBag.go @@ -174,53 +174,25 @@ func (mb *MutableBag) Reset() { } } -// Merge combines an array of bags into the current bag. -// -// The individual bags may not contain any conflicting attribute -// values. If that happens, then the merge fails and no mutation -// will have occurred to the current bag. +// Merge combines an array of bags into the current bag. If the current bag already defines +// a particular attribute, it keeps its value and is not overwritten. // // Note that this does a 'shallow' merge. Only the value defined explicitly in the // mutable bags themselves, and not in any of their parents, are considered. -func (mb *MutableBag) Merge(bags ...*MutableBag) error { - // first step is to make sure there are no redundant definitions of the same attribute in the incoming bags - if len(bags) > 1 { - keys := make(map[string]bool) - for _, bag := range bags { - if bag == nil { - continue - } - - for k := range bag.values { - if keys[k] { - return fmt.Errorf("conflicting value for attribute %s", k) - } - keys[k] = true - } - } - } - +func (mb *MutableBag) Merge(bag *MutableBag) { // get the known symbols for the target bag names := make(map[string]bool) for _, name := range mb.Names() { names[name] = true } - for _, bag := range bags { - if bag == nil { - continue - } - - for k, v := range bag.values { - // the input bags cannot override values already in the destination bag - _, found := names[k] - if !found { - mb.values[k] = copyValue(v) - } + for k, v := range bag.values { + // the input bags cannot override values already in the destination bag + _, found := names[k] + if !found { + mb.values[k] = copyValue(v) } } - - return nil } // ToProto fills-in an Attributes proto based on the content of the bag. diff --git a/mixer/pkg/attribute/protoBag.go b/mixer/pkg/attribute/protoBag.go index a5702d57bbe1..af7a2ee7425d 100644 --- a/mixer/pkg/attribute/protoBag.go +++ b/mixer/pkg/attribute/protoBag.go @@ -156,7 +156,11 @@ func (pb *ProtoBag) ClearReferencedAttributes() { // ResetReferencedAttributes sets the list of referenced attributes being tracked by this bag func (pb *ProtoBag) ResetReferencedAttributes(snap ReferencedAttributeSnapshot) { - pb.referencedAttrs = snap.referencedAttrs + ra := make(map[attributeRef]mixerpb.ReferencedAttributes_Condition, len(snap.referencedAttrs)) + for k, v := range snap.referencedAttrs { + ra[k] = v + } + pb.referencedAttrs = ra } // SnapshotReferencedAttributes grabs a snapshot of the currently referenced attributes diff --git a/mixer/pkg/il/testing/tests.go b/mixer/pkg/il/testing/tests.go index a4c432ef8d14..64850d89fa51 100644 --- a/mixer/pkg/il/testing/tests.go +++ b/mixer/pkg/il/testing/tests.go @@ -502,25 +502,25 @@ end`, conf: exprEvalAttrs, }, { - E: `target.ip| ip("10.1.12.3")`, + E: `destination.ip| ip("10.1.12.3")`, Type: descriptor.IP_ADDRESS, I: map[string]interface{}{}, R: net.ParseIP("10.1.12.3"), - Referenced: []string{"target.ip"}, + Referenced: []string{"destination.ip"}, conf: exprEvalAttrs, }, { - E: `target.ip| ip(2)`, + E: `destination.ip| ip(2)`, Type: descriptor.IP_ADDRESS, I: map[string]interface{}{ - "target.ip": "", + "destination.ip": "", }, CompileErr: "ip(2) arg 1 (2) typeError got INT64, expected STRING", AstErr: "input to 'ip' func was not a string", conf: exprEvalAttrs, }, { - E: `target.ip| ip("10.1.12")`, + E: `destination.ip| ip("10.1.12")`, Type: descriptor.IP_ADDRESS, I: map[string]interface{}{}, Err: "could not convert 10.1.12 to IP_ADDRESS", @@ -3462,7 +3462,7 @@ var exprEvalAttrs = map[string]*pb.AttributeManifest_AttributeInfo{ "headername": { ValueType: descriptor.STRING, }, - "target.ip": { + "destination.ip": { ValueType: descriptor.IP_ADDRESS, }, "servicename": { diff --git a/mixer/pkg/perf/setup_test.go b/mixer/pkg/perf/setup_test.go index bcc22188b6ad..54444af97fae 100644 --- a/mixer/pkg/perf/setup_test.go +++ b/mixer/pkg/perf/setup_test.go @@ -123,13 +123,13 @@ load: iterations: 100 requests: - attributes: - target.name: somesrvcname + destination.name: somesrvcname type: basicReport - attributes: - target.name: cvd + destination.name: cvd type: basicReport - attributes: - target.name: somesrvcname + destination.name: somesrvcname type: basicCheck `, setup: Setup{ @@ -144,14 +144,14 @@ load: Multiplier: 100, Requests: []Request{ BasicReport{ - Attributes: map[string]interface{}{"target.name": "somesrvcname"}, + Attributes: map[string]interface{}{"destination.name": "somesrvcname"}, }, BasicReport{ - Attributes: map[string]interface{}{"target.name": "cvd"}, + Attributes: map[string]interface{}{"destination.name": "cvd"}, }, BasicCheck{ Attributes: map[string]interface{}{ - "target.name": "somesrvcname", + "destination.name": "somesrvcname", }, }, }, diff --git a/mixer/pkg/perf/standard.go b/mixer/pkg/perf/standard.go index 7f3750baf9f6..6f6e651bb13d 100644 --- a/mixer/pkg/perf/standard.go +++ b/mixer/pkg/perf/standard.go @@ -75,7 +75,7 @@ spec: attributes: source.name: value_type: STRING - target.name: + destination.name: value_type: STRING response.count: value_type: INT64 @@ -108,7 +108,7 @@ spec: value: "2" dimensions: source: source.name | "mysrc" - target_ip: target.name | "mytarget" + target_ip: destination.name | "mytarget" --- @@ -118,7 +118,7 @@ metadata: name: rule1 namespace: istio-system spec: - selector: match(target.name, "*") + selector: match(destination.name, "*") actions: - handler: fakeHandlerConfig.fakeHandler instances: diff --git a/mixer/pkg/runtime/controller_test.go b/mixer/pkg/runtime/controller_test.go index f2a1530aac96..a2b802f7a67b 100644 --- a/mixer/pkg/runtime/controller_test.go +++ b/mixer/pkg/runtime/controller_test.go @@ -186,7 +186,7 @@ func TestController_workflow(t *testing.T) { } configState := map[store.Key]*store.Resource{ {RulesKind, DefaultConfigNamespace, "r1"}: {Spec: &cpb.Rule{ - Match: "target.service == \"abc\"", + Match: "destination.service == \"abc\"", Actions: []*cpb.Action{ { Handler: "a1.AA." + DefaultConfigNamespace, @@ -251,7 +251,7 @@ func TestController_workflow(t *testing.T) { { Key: store.Key{RulesKind, DefaultConfigNamespace, "r2"}, Value: &store.Resource{Spec: &cpb.Rule{ - Match: "target.service == \"bcd\"", + Match: "destination.service == \"bcd\"", Actions: []*cpb.Action{ { Handler: "a1.AA." + DefaultConfigNamespace, diff --git a/mixer/pkg/runtime/dispatcher.go b/mixer/pkg/runtime/dispatcher.go index 3fea112a8f4b..52f627629baf 100644 --- a/mixer/pkg/runtime/dispatcher.go +++ b/mixer/pkg/runtime/dispatcher.go @@ -300,12 +300,9 @@ func (m *dispatcher) Preprocess(ctx context.Context, requestBag attribute.Bag, r if err == nil { lock.Lock() defer lock.Unlock() - err = responseBag.Merge(mBag) - - if err != nil { - log.Infof("Attributes merging failed %v", err) + if mBag != nil { + responseBag.Merge(mBag) } - } return &result{err, nil, call} }) diff --git a/mixer/pkg/runtime/resolver.go b/mixer/pkg/runtime/resolver.go index 786ec994938d..7cf5a31907e1 100644 --- a/mixer/pkg/runtime/resolver.go +++ b/mixer/pkg/runtime/resolver.go @@ -54,7 +54,7 @@ type resolver struct { evaluator expr.Evaluator // identityAttribute defines which configuration scopes apply to a request. - // default: target.service + // default: destination.service // The value of this attribute is expected to be a hostname of form "svc.$ns.suffix" identityAttribute string @@ -106,7 +106,7 @@ const ( // Resolve resolves the in memory configuration to a set of actions based on request attributes. // Resolution is performed in the following order // 1. Check rules from the defaultConfigNamespace -- these rules always apply -// 2. Check rules from the target.service namespace +// 2. Check rules from the destination.service namespace func (r *resolver) Resolve(attrs attribute.Bag, variety adptTmpl.TemplateVariety) (ra Actions, err error) { nselected := 0 target := "unknown" diff --git a/mixer/pkg/runtime2/dispatcher/dispatcher.go b/mixer/pkg/runtime2/dispatcher/dispatcher.go index f0a65f53fa41..494aa8228d2f 100644 --- a/mixer/pkg/runtime2/dispatcher/dispatcher.go +++ b/mixer/pkg/runtime2/dispatcher/dispatcher.go @@ -298,10 +298,8 @@ func (d *Dispatcher) dispatch(session *session) error { st = state.quotaResult.Status case tpb.TEMPLATE_VARIETY_ATTRIBUTE_GENERATOR: - e := session.responseBag.Merge(state.outputBag) - if e != nil { - log.Infof("Attributes merging failed %v", err) - err = e + if state.outputBag != nil { + session.responseBag.Merge(state.outputBag) } } diff --git a/mixer/pkg/runtime2/dispatcher/dispatcher_test.go b/mixer/pkg/runtime2/dispatcher/dispatcher_test.go index 2fce40e2dd68..3b80a115809e 100644 --- a/mixer/pkg/runtime2/dispatcher/dispatcher_test.go +++ b/mixer/pkg/runtime2/dispatcher/dispatcher_test.go @@ -634,8 +634,8 @@ ident : dest.istio-system data.RuleCheck1WithMatchClause, }, attr: map[string]interface{}{ - "ident": "dest.istio-system", - "target.name": "barf", // "foo*" is expected + "ident": "dest.istio-system", + "destination.name": "barf", // "foo*" is expected }, variety: tpb.TEMPLATE_VARIETY_CHECK, log: ``, // log should be empty diff --git a/mixer/pkg/runtime2/routing/builder_test.go b/mixer/pkg/runtime2/routing/builder_test.go index f75c9688def4..952d215ccdd7 100644 --- a/mixer/pkg/runtime2/routing/builder_test.go +++ b/mixer/pkg/runtime2/routing/builder_test.go @@ -128,7 +128,7 @@ ID: 1 [#0] istio-system {NS} [#0] hcheck1.acheck.istio-system {H} [#0] - Condition: match(target.name, "foo*") + Condition: match(destination.name, "foo*") [#0] icheck1.tcheck.istio-system {I} `, }, @@ -169,7 +169,7 @@ ID: 1 [#0] istio-system {NS} [#0] hcheck1.acheck.istio-system {H} [#0] - Condition: match(target.name, "foo*") + Condition: match(destination.name, "foo*") [#0] icheck1.tcheck.istio-system {I} [#1] icheck2.tcheck.istio-system {I} `, @@ -193,13 +193,13 @@ ID: 1 [#0] istio-system {NS} [#0] hcheck1.acheck.istio-system {H} [#0] - Condition: match(target.name, "foo*") - [#0] icheck1.tcheck.istio-system {I} - [#1] icheck2.tcheck.istio-system {I} - [#1] - Condition: target.name.startsWith("foo") + Condition: destination.name.startsWith("foo") [#0] icheck2.tcheck.istio-system {I} [#1] icheck3.tcheck.istio-system {I} + [#1] + Condition: match(destination.name, "foo*") + [#0] icheck1.tcheck.istio-system {I} + [#1] icheck2.tcheck.istio-system {I} `, }, @@ -251,7 +251,7 @@ ID: 1 Condition: [#0] icheck1.tcheck.istio-system {I} [#1] - Condition: target.name.startsWith("foo") + Condition: destination.name.startsWith("foo") [#0] icheck2.tcheck.istio-system {I} [#1] icheck3.tcheck.istio-system {I} `, diff --git a/mixer/pkg/runtime2/routing/table_test.go b/mixer/pkg/runtime2/routing/table_test.go index ce3caa4ddf3c..d87131a3afea 100644 --- a/mixer/pkg/runtime2/routing/table_test.go +++ b/mixer/pkg/runtime2/routing/table_test.go @@ -162,7 +162,7 @@ func TestInputs_Matches(t *testing.T) { // Value is in the bag, but does match the condition. bag = attribute.GetFakeMutableBagForTesting(map[string]interface{}{ - "target.name": "barfoo", + "destination.name": "barfoo", }) if i.Matches(bag) { t.Fatal("The group shouldn't have matched") @@ -170,7 +170,7 @@ func TestInputs_Matches(t *testing.T) { // Value is in the bag, and matches the condition bag = attribute.GetFakeMutableBagForTesting(map[string]interface{}{ - "target.name": "foobar", + "destination.name": "foobar", }) if !i.Matches(bag) { t.Fatal("The group should have matched") diff --git a/mixer/pkg/runtime2/testing/data/data.go b/mixer/pkg/runtime2/testing/data/data.go index 5cefb986938b..a44e8ac93016 100644 --- a/mixer/pkg/runtime2/testing/data/data.go +++ b/mixer/pkg/runtime2/testing/data/data.go @@ -27,7 +27,7 @@ spec: attributes: source.name: value_type: STRING - target.name: + destination.name: value_type: STRING response.count: value_type: INT64 @@ -327,7 +327,7 @@ metadata: name: rcheck1 namespace: istio-system spec: - match: match(target.name, "foo*") + match: match(destination.name, "foo*") actions: - handler: hcheck1.acheck instances: @@ -342,7 +342,7 @@ metadata: name: rcheck1 namespace: istio-system spec: - match: match(target.name, "foo*") + match: match(destination.name, "foo*") actions: - handler: hcheck1.acheck instances: @@ -372,7 +372,7 @@ metadata: name: rcheck1 namespace: istio-system spec: - selector: target.name + selector: destination.name actions: - handler: hcheck1.acheck instances: @@ -416,7 +416,7 @@ metadata: name: rcheck2 namespace: istio-system spec: - match: target.name.startsWith("foo") + match: destination.name.startsWith("foo") actions: - handler: hcheck1.acheck instances: diff --git a/mixer/pkg/server/server_test.go b/mixer/pkg/server/server_test.go index 672ad37f91ff..440bd8e49f69 100644 --- a/mixer/pkg/server/server_test.go +++ b/mixer/pkg/server/server_test.go @@ -53,7 +53,7 @@ spec: attributes: source.name: value_type: STRING - target.name: + destination.name: value_type: STRING response.count: value_type: INT64 @@ -85,7 +85,7 @@ spec: value: "2" dimensions: source: source.name | "mysrc" - target_ip: target.name | "mytarget" + target_ip: destination.name | "mytarget" --- @@ -95,7 +95,7 @@ metadata: name: rule1 namespace: istio-system spec: - selector: match(target.name, "*") + selector: match(destination.name, "*") actions: - handler: fakeHandlerConfig.fakeHandler instances: diff --git a/mixer/template/authorization/authorization.pb.html b/mixer/template/authorization/authorization.pb.html index bbeca5921ddc..ca3d495430c8 100644 --- a/mixer/template/authorization/authorization.pb.html +++ b/mixer/template/authorization/authorization.pb.html @@ -132,8 +132,8 @@

Template

properties: iss: request.auth.token["iss"] action: - namespace: target.namespace | "default" - service: target.service | "" + namespace: destination.namespace | "default" + service: destination.service | "" path: request.path | "/" method: request.method | "post" properties: diff --git a/mixer/template/authorization/template.proto b/mixer/template/authorization/template.proto index a18f9e49078c..aa647a5f1556 100644 --- a/mixer/template/authorization/template.proto +++ b/mixer/template/authorization/template.proto @@ -78,8 +78,8 @@ message Action { // properties: // iss: request.auth.token["iss"] // action: -// namespace: target.namespace | "default" -// service: target.service | "" +// namespace: destination.namespace | "default" +// service: destination.service | "" // path: request.path | "/" // method: request.method | "post" // properties: diff --git a/mixer/template/authorization/template_handler.gen.go b/mixer/template/authorization/template_handler.gen.go index 7dd1d81c8ddf..50daef5f6361 100644 --- a/mixer/template/authorization/template_handler.gen.go +++ b/mixer/template/authorization/template_handler.gen.go @@ -53,8 +53,8 @@ const TemplateName = "authorization" // properties: // iss: request.auth.token["iss"] // action: -// namespace: target.namespace | "default" -// service: target.service | "" +// namespace: destination.namespace | "default" +// service: destination.service | "" // path: request.path | "/" // method: request.method | "post" // properties: diff --git a/mixer/template/authorization/template_instance.pb.go b/mixer/template/authorization/template_instance.pb.go index 3d41ee20897a..7170ebe6c7fa 100644 --- a/mixer/template/authorization/template_instance.pb.go +++ b/mixer/template/authorization/template_instance.pb.go @@ -65,8 +65,8 @@ const _ = proto.GoGoProtoPackageIsVersion2 // please upgrade the proto package // properties: // iss: request.auth.token["iss"] // action: -// namespace: target.namespace | "default" -// service: target.service | "" +// namespace: destination.namespace | "default" +// service: destination.service | "" // path: request.path | "/" // method: request.method | "post" // properties: diff --git a/mixer/template/sample/template.gen_test.go b/mixer/template/sample/template.gen_test.go index f605006655d0..6ce5b814e0c6 100644 --- a/mixer/template/sample/template.gen_test.go +++ b/mixer/template/sample/template.gen_test.go @@ -682,7 +682,7 @@ duration: source.duration dimensions: source: source.string target: source.string - env: target.string + env: destination.string res1: value: source.int64 int64Primitive: source.int64 @@ -694,7 +694,7 @@ res1: dimensions: source: source.string target: source.string - env: target.string + env: destination.string `, cstrParam: &sample_quota.InstanceParam{}, wantType: &sample_quota.Type{ diff --git a/mixer/testdata/config/attributes.yaml b/mixer/testdata/config/attributes.yaml index a500623b5b0f..72b61f4df3e5 100644 --- a/mixer/testdata/config/attributes.yaml +++ b/mixer/testdata/config/attributes.yaml @@ -47,8 +47,6 @@ spec: valueType: STRING source.user: valueType: STRING - target.uid: - valueType: STRING destination.uid: valueType: STRING connection.id: @@ -105,17 +103,6 @@ spec: valueType: STRING source.serviceAccount: valueType: STRING - target.ip: - valueType: IP_ADDRESS - target.labels: - valueType: STRING_MAP - target.name: - valueType: STRING - target.namespace: - valueType: STRING - target.service: - valueType: STRING - target.serviceAccount: valueType: STRING destination.ip: valueType: IP_ADDRESS diff --git a/mixer/testdata/configroot/scopes/global/descriptors.yml b/mixer/testdata/configroot/scopes/global/descriptors.yml index afedc1d41f9d..dee5cf8f2db0 100644 --- a/mixer/testdata/configroot/scopes/global/descriptors.yml +++ b/mixer/testdata/configroot/scopes/global/descriptors.yml @@ -16,18 +16,6 @@ manifests: valueType: STRING source.serviceAccount: valueType: STRING - target.ip: - valueType: IP_ADDRESS - target.labels: - valueType: STRING_MAP - target.name: - valueType: STRING - target.namespace: - valueType: STRING - target.service: - valueType: STRING - target.serviceAccount: - valueType: STRING destination.ip: valueType: IP_ADDRESS destination.labels: @@ -89,7 +77,7 @@ manifests: valueType: STRING source.user: valueType: STRING - target.uid: + destination.uid: valueType: STRING connection.id: valueType: STRING diff --git a/samples/bookinfo/kube/mixer-rule-ratings-denial.yaml b/samples/bookinfo/kube/mixer-rule-ratings-denial.yaml index 0578f916c8cf..d7324aca2627 100644 --- a/samples/bookinfo/kube/mixer-rule-ratings-denial.yaml +++ b/samples/bookinfo/kube/mixer-rule-ratings-denial.yaml @@ -22,7 +22,7 @@ metadata: name: denyreviewsv3 namespace: istio-system spec: - #FIXME match: target.labels["app"]=="productpage" && request.headers["x-user"] == "" + #FIXME match: destination.labels["app"]=="productpage" && request.headers["x-user"] == "" match: request.headers["x-user"] == "" actions: - handler: handler.denier.istio-system diff --git a/tools/rules.yml b/tools/rules.yml index eafbd4840db1..509f85df4495 100644 --- a/tools/rules.yml +++ b/tools/rules.yml @@ -18,19 +18,19 @@ rules: value: "1" labels: source: source.labels["app"] | "unknown" - target: target.service | "unknown" - service: target.labels["app"] | "unknown" + target: destination.service | "unknown" + service: destination.labels["app"] | "unknown" method: request.path | "unknown" - version: target.labels["version"] | "unknown" + version: destination.labels["version"] | "unknown" response_code: response.code | 200 - descriptor_name: request_duration value: response.duration | "0ms" labels: source: source.labels["app"] | "unknown" - target: target.service | "unknown" - service: target.labels["app"] | "unknown" + target: destination.service | "unknown" + service: destination.labels["app"] | "unknown" method: request.path | "unknown" - version: target.labels["version"] | "unknown" + version: destination.labels["version"] | "unknown" response_code: response.code | 200 - kind: access-logs params: