Fix 500 in attributes adapter. Add a "rejector". Make the attributes adapter set "success" but not return "false" upon error. Also add a "rejector" adapter that just checks for the string "false".
diff --git a/adapter/BUILD b/adapter/BUILD index 68a50d4..f42b913 100644 --- a/adapter/BUILD +++ b/adapter/BUILD
@@ -11,6 +11,7 @@ "apigeeReport.go", "applications.go", "products.go", + "rejecter.go", ], deps = [ "//adapter/config:go_default_library", @@ -27,6 +28,7 @@ "apigeeKeyAttributes_test.go", "apigeeKeyChecker_test.go", "apigeeReport_test.go", + "rejecter_test.go", ], library = ":go_default_library", deps = [
diff --git a/adapter/apigee.go b/adapter/apigee.go index a1f7b75..a35c438 100644 --- a/adapter/apigee.go +++ b/adapter/apigee.go
@@ -25,6 +25,7 @@ // aspect of the Apigee functionality. func Register(r adapter.Registrar) { r.RegisterListsBuilder(newKeyCheckBuilder()) + r.RegisterListsBuilder(newRejectBuilder()) //r.RegisterApplicationLogsBuilder(newReportBuilder()) r.RegisterAccessLogsBuilder(newReportBuilder()) r.RegisterAttributesGeneratorBuilder(newKeyAttrsBuilder())
diff --git a/adapter/apigeeKeyAttributes.go b/adapter/apigeeKeyAttributes.go index db93ed5..6971f98 100644 --- a/adapter/apigeeKeyAttributes.go +++ b/adapter/apigeeKeyAttributes.go
@@ -74,13 +74,18 @@ } func (g *keyAttrsGenerator) Generate(in map[string]interface{}) (map[string]interface{}, error) { + + out := make(map[string]interface{}) + out[successParam] = false + out[successStringParam] = "false" + key := getString(in, keyParam) if key == "" { - return nil, fmt.Errorf("Cannot verify API key: value of \"%s\" not found", keyParam) + return out, nil } path := getString(in, pathParam) if path == "" { - return nil, fmt.Errorf("Cannot verify API key: value of \"%s\" not found", pathParam) + return out, nil } // Look up API key from cache, making HTTP request if necessary @@ -102,7 +107,6 @@ } // TODO match API products by path - out := make(map[string]interface{}) out[successParam] = success out[successStringParam] = strconv.FormatBool(success) if app.valid {
diff --git a/adapter/apigeeKeyAttributes_test.go b/adapter/apigeeKeyAttributes_test.go index b2b5883..28d9438 100644 --- a/adapter/apigeeKeyAttributes_test.go +++ b/adapter/apigeeKeyAttributes_test.go
@@ -23,6 +23,74 @@ "github.com/apid/istioApigeeAdapter/mock" ) +func TestMissingKey(t *testing.T) { + cfg := &config.VerifyKeyParams{ + Organization: "foo", + Environment: "test", + VerificationURL: "http://" + mockServer.Address(), + } + + builder := newKeyAttrsBuilder() + ce := builder.ValidateConfig(cfg) + if ce != nil { + t.Fatalf("Error validating config: %s", ce) + } + + ag, err := builder.BuildAttributesGenerator(mockEnv, cfg) + if err != nil { + t.Fatalf("Error creating aspect: %s", err) + } + defer ag.Close() + + in := make(map[string]interface{}) + + out, err := ag.Generate(in) + if err != nil { + t.Fatalf("Error checking parameters: %s", err) + } + if out[successParam] != false { + t.Fatal("Expected success to be false") + } + if out[successStringParam] != "false" { + t.Fatal("Expected success string to be false") + } +} + +func TestEmptyKey(t *testing.T) { + cfg := &config.VerifyKeyParams{ + Organization: "foo", + Environment: "test", + VerificationURL: "http://" + mockServer.Address(), + } + + builder := newKeyAttrsBuilder() + ce := builder.ValidateConfig(cfg) + if ce != nil { + t.Fatalf("Error validating config: %s", ce) + } + + ag, err := builder.BuildAttributesGenerator(mockEnv, cfg) + if err != nil { + t.Fatalf("Error creating aspect: %s", err) + } + defer ag.Close() + + in := make(map[string]interface{}) + in[keyParam] = "99999" + in[pathParam] = "/" + + out, err := ag.Generate(in) + if err != nil { + t.Fatalf("Error checking parameters: %s", err) + } + if out[successParam] != false { + t.Fatal("Expected success to be false") + } + if out[successStringParam] != "false" { + t.Fatal("Expected success string to be false") + } +} + func TestInvalidKeyAttributes(t *testing.T) { cfg := &config.VerifyKeyParams{ Organization: "foo",
diff --git a/adapter/config/config.proto b/adapter/config/config.proto index b069fbc..d51106b 100644 --- a/adapter/config/config.proto +++ b/adapter/config/config.proto
@@ -29,6 +29,11 @@ string verificationURL = 3; } +message RejectParams { + // An optional error message to return + string message = 1; +} + message ReportParams { // The name of the Apigee organization -- required string organization = 1;
diff --git a/adapter/rejecter.go b/adapter/rejecter.go new file mode 100644 index 0000000..f22c483 --- /dev/null +++ b/adapter/rejecter.go
@@ -0,0 +1,66 @@ +/* +Copyright 2017 Google Inc. + +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 adapter + +import ( + "strconv" + + "github.com/apid/istioApigeeAdapter/adapter/config" + "istio.io/mixer/pkg/adapter" +) + +const ( + rejectName = "rejecter" + rejectDesc = "Reject unless the expression is 'true'" +) + +var rejectConf = &config.RejectParams{} + +type rejectBuilder struct { + adapter.DefaultBuilder +} + +type rejecter struct { + message string +} + +func newRejectBuilder() adapter.ListsBuilder { + return rejectBuilder{ + adapter.NewDefaultBuilder(rejectName, rejectDesc, rejectConf), + } +} + +func (b rejectBuilder) NewListsAspect(env adapter.Env, c adapter.Config) (adapter.ListsAspect, error) { + cfg := c.(*config.RejectParams) + r := &rejecter{ + message: cfg.Message, + } + return r, nil +} + +func (l *rejecter) Close() error { + return nil +} + +func (l *rejecter) CheckList(symbol string) (bool, error) { + isTrue, err := strconv.ParseBool(symbol) + if err != nil { + // Error here means something like an empty string + return false, nil + } + return isTrue, nil +}
diff --git a/adapter/rejecter_test.go b/adapter/rejecter_test.go new file mode 100644 index 0000000..1c671cc --- /dev/null +++ b/adapter/rejecter_test.go
@@ -0,0 +1,89 @@ +/* +Copyright 2017 Google Inc. + +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 adapter + +import ( + "testing" + + "github.com/apid/istioApigeeAdapter/adapter/config" +) + +func TestRejectFalseness(t *testing.T) { + cfg := &config.RejectParams{} + builder := newRejectBuilder() + ce := builder.ValidateConfig(cfg) + if ce != nil { + t.Fatal("Config should validate") + } + aspect, err := builder.NewListsAspect(mockEnv, cfg) + if err != nil { + t.Fatalf("Error creating aspect: %s", err) + } + defer aspect.Close() + + result, err := aspect.CheckList("false") + if err != nil { + t.Fatalf("Error on list check: %s", err) + } + if result { + t.Fatal("List check returned true and should have failed") + } +} + +func TestRejectEmpty(t *testing.T) { + cfg := &config.RejectParams{} + builder := newRejectBuilder() + ce := builder.ValidateConfig(cfg) + if ce != nil { + t.Fatal("Config should validate") + } + aspect, err := builder.NewListsAspect(mockEnv, cfg) + if err != nil { + t.Fatalf("Error creating aspect: %s", err) + } + defer aspect.Close() + + result, err := aspect.CheckList("") + if err != nil { + t.Fatalf("Error on list check: %s", err) + } + if result { + t.Fatal("List check returned true and should have failed") + } +} + +func TestRejectTruthy(t *testing.T) { + cfg := &config.RejectParams{} + builder := newRejectBuilder() + ce := builder.ValidateConfig(cfg) + if ce != nil { + t.Fatal("Config should validate") + } + aspect, err := builder.NewListsAspect(mockEnv, cfg) + if err != nil { + t.Fatalf("Error creating aspect: %s", err) + } + defer aspect.Close() + + result, err := aspect.CheckList("true") + if err != nil { + t.Fatalf("Error on list check: %s", err) + } + if !result { + t.Fatal("List check returned false") + } +}