From b76e7d25bec020666b5feaa7f9464873d1d05cdc Mon Sep 17 00:00:00 2001 From: Alex Schneider Date: Fri, 10 Mar 2023 12:28:21 +0100 Subject: [PATCH] Deprecations (#734) * Deprecation structure * Do not add wildcard label * Fix panics * Do not ignore errors * Add deprecated items * Add logger * Remove unnecessary assignment to the blank identifier * Cleanup * Update config/configload/deprecated_test.go Co-authored-by: Johannes Koch <53434855+johakoch@users.noreply.github.com> * store granted permissions and required permission in request context under beta_ prefixed key, too * Lowercase error message --------- Co-authored-by: Johannes Koch <53434855+johakoch@users.noreply.github.com> Co-authored-by: Johannes Koch --- command/verify.go | 2 +- config/configload/deprecated.go | 139 ++++++++++++++++++++++++ config/configload/deprecated_test.go | 83 ++++++++++++++ config/configload/error_handler.go | 6 +- config/configload/error_handler_test.go | 2 +- config/configload/load.go | 17 +-- config/configload/validate.go | 4 +- eval/context.go | 2 + main.go | 6 +- server/http_integration_test.go | 2 +- 10 files changed, 245 insertions(+), 18 deletions(-) create mode 100644 config/configload/deprecated.go create mode 100644 config/configload/deprecated_test.go diff --git a/command/verify.go b/command/verify.go index a784df4ba..8759524e5 100644 --- a/command/verify.go +++ b/command/verify.go @@ -20,7 +20,7 @@ func NewVerify() *Verify { } func (v Verify) Execute(args Args, conf *config.Couper, logger *logrus.Entry) error { - cf, err := configload.LoadFiles(args, conf.Environment) + cf, err := configload.LoadFiles(args, conf.Environment, logger) if diags, ok := err.(hcl.Diagnostics); ok { for _, diag := range diags { logger.WithError(diag).Error() diff --git a/config/configload/deprecated.go b/config/configload/deprecated.go new file mode 100644 index 000000000..bb98beaa5 --- /dev/null +++ b/config/configload/deprecated.go @@ -0,0 +1,139 @@ +package configload + +import ( + "github.com/hashicorp/hcl/v2/hclsyntax" + "github.com/sirupsen/logrus" +) + +type deprecated struct { + newName string + version string +} + +type ( + attributesList map[string]deprecated + blocksList map[string]deprecated + labelsList map[string]deprecated +) + +var ( + deprecatedAttributes attributesList + deprecatedBlocks blocksList + deprecatedLabels labelsList +) + +func init() { + deprecatedAttributes = make(map[string]deprecated) + deprecatedBlocks = make(map[string]deprecated) + deprecatedLabels = make(map[string]deprecated) + + // Deprecated attributes: + // deprecatedAttributes["..."] = deprecated{"...", "..."} + deprecatedAttributes["beta_permissions_claim"] = deprecated{"permissions_claim", "1.13"} + deprecatedAttributes["beta_permissions_map"] = deprecated{"permissions_map", "1.13"} + deprecatedAttributes["beta_permissions_map_file"] = deprecated{"permissions_map_file", "1.13"} + deprecatedAttributes["beta_required_permission"] = deprecated{"required_permission", "1.13"} + deprecatedAttributes["beta_roles_claim"] = deprecated{"roles_claim", "1.13"} + deprecatedAttributes["beta_roles_map"] = deprecated{"roles_map", "1.13"} + deprecatedAttributes["beta_roles_file"] = deprecated{"roles_map_file", "1.13"} + + // Deprecated blocks: + // deprecatedBlocks["..."] = deprecated{"...", "..."} + + // Deprecated labels: + // deprecatedLabels["..."] = deprecated{"...", "..."} + deprecatedLabels["beta_insufficient_permissions"] = deprecated{"insufficient_permissions", "1.13"} + + // TODO with 1.13, also remove + // ctxAcMap["beta_"+grantedPermissions] = seetie.GoToValue(gp) + // ctxAcMap["beta_"+requiredPermission] = seetie.GoToValue(rp) + // in eval/context.go newVariable() +} + +func deprecate(bodies []*hclsyntax.Body, logger *logrus.Entry) { + for _, body := range bodies { + deprecateBody(body, logger) + } +} + +func deprecateBody(body *hclsyntax.Body, logger *logrus.Entry) { + if body == nil { + return + } + + body.Attributes = deprecateAttributes(body.Attributes, logger) + + deprecateBlocks(body.Blocks, logger) +} + +func deprecateAttributes(attributes hclsyntax.Attributes, logger *logrus.Entry) hclsyntax.Attributes { + attrs := make(hclsyntax.Attributes) + + for _, attr := range attributes { + name := attr.Name + + if rename, exists := deprecatedAttributes[name]; exists { + rename.log("attribute", name, logger) + + name = rename.newName + } + + attrs[name] = attr + } + + return attrs +} + +func deprecateBlocks(blocks hclsyntax.Blocks, logger *logrus.Entry) { + for _, block := range blocks { + block.Labels = deprecateLabels(block, logger) + + if rename, exists := deprecatedBlocks[block.Type]; exists { + rename.log("block", block.Type, logger) + + block.Type = rename.newName + } + + deprecateBody(block.Body, logger) + } +} + +func deprecateLabels(block *hclsyntax.Block, logger *logrus.Entry) []string { + var ( + err error + labels []string = block.Labels + renamed []string + ) + + if block.Type == errorHandler { + labels, err = newKindsFromLabels(block, false) + + if err != nil { + return block.Labels + } + } + + for _, label := range labels { + name := label + + if rename, exists := deprecatedLabels[label]; exists { + rename.log("label", label, logger) + + name = rename.newName + } + + renamed = append(renamed, name) + } + + return renamed +} + +// In some test cases the logger is , but not in production code. +func (d deprecated) log(name, old string, logger *logrus.Entry) { + if logger != nil { + logger.Warnf( + "replacing %s %q with %q; as of Couper version %s, the old value is no longer supported", + name, old, d.newName, d.version, + ) + } +} diff --git a/config/configload/deprecated_test.go b/config/configload/deprecated_test.go new file mode 100644 index 000000000..c6d286282 --- /dev/null +++ b/config/configload/deprecated_test.go @@ -0,0 +1,83 @@ +package configload + +import ( + "context" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/hashicorp/hcl/v2/hclsyntax" + + "github.com/avenga/couper/config/parser" + "github.com/avenga/couper/internal/test" +) + +func Test_deprecated(t *testing.T) { + // Insert test data: + deprecatedAttributes["couper_test_attribute"] = deprecated{"couper_new_attribute", "1.23"} + deprecatedBlocks["couper_test_block"] = deprecated{"couper_new_block", "1.23"} + deprecatedLabels["couper_test_label"] = deprecated{"couper_new_label", "1.23"} + + src := []byte(` +error_handler "x" "couper_test_label" "abc couper_test_label def" "y" { + couper_test_attribute = true + couper_test_block { + } +} +`) + + body, err := parser.Load(src, "test.go") + if err != nil { + t.Fatalf("%s", err) + } + + logger, hook := test.NewLogger() + hook.Reset() + + deprecate([]*hclsyntax.Body{body}, logger.WithContext(context.TODO())) + + if len(body.Blocks) != 1 { + t.Fatal("Unexpected number of blocks given") + } + + if len(body.Blocks[0].Body.Attributes) != 1 { + t.Fatal("Unexpected number of attributes given") + } + + if attr, exists := body.Blocks[0].Body.Attributes["couper_new_attribute"]; !exists { + t.Error("Missing 'couper_new_attribute' attribute") + } else if attr.Name != "couper_test_attribute" { + t.Errorf("Unexpected attribute name given: '%s'", attr.Name) + } + + if body.Blocks[0].Body.Blocks[0].Type != "couper_new_block" { + t.Errorf("Expected 'couper_new_block' block name, got '%s'", body.Blocks[0].Type) + } + + expLabels := []string{"x", "couper_new_label", "abc", "couper_new_label", "def", "y"} + if !cmp.Equal(expLabels, body.Blocks[0].Labels) { + t.Errorf("Expected\n%#v, got:\n%#v", expLabels, body.Blocks[0].Labels) + } + + entries := hook.AllEntries() + if len(entries) != 4 { + t.Fatal("Unexpected number of log entries given") + } + + exp := `replacing label "couper_test_label" with "couper_new_label"; as of Couper version 1.23, the old value is no longer supported` + if entries[0].Message != exp { + t.Errorf("Expected\n%#v, got:\n%#v", exp, entries[0].Message) + } + if entries[1].Message != exp { + t.Errorf("Expected\n%#v, got:\n%#v", exp, entries[0].Message) + } + + exp = `replacing attribute "couper_test_attribute" with "couper_new_attribute"; as of Couper version 1.23, the old value is no longer supported` + if entries[2].Message != exp { + t.Errorf("Expected\n%#v, got:\n%#v", exp, entries[0].Message) + } + + exp = `replacing block "couper_test_block" with "couper_new_block"; as of Couper version 1.23, the old value is no longer supported` + if entries[3].Message != exp { + t.Errorf("Expected\n%#v, got:\n%#v", exp, entries[0].Message) + } +} diff --git a/config/configload/error_handler.go b/config/configload/error_handler.go index a118ad03d..689c78f8f 100644 --- a/config/configload/error_handler.go +++ b/config/configload/error_handler.go @@ -72,7 +72,7 @@ func newErrorHandlerContent(content *hclsyntax.Body) (map[string]struct{}, []kin var kindContents []kindContent for _, block := range hclbody.BlocksOfType(content, errorHandler) { - kinds, err := newKindsFromLabels(block) + kinds, err := newKindsFromLabels(block, true) if err != nil { return nil, nil, err } @@ -117,7 +117,7 @@ func newErrorHandlerContent(content *hclsyntax.Body) (map[string]struct{}, []kin const errorHandlerLabelSep = " " // newKindsFromLabels reads two possible kind formats and returns them per slice entry. -func newKindsFromLabels(block *hclsyntax.Block) ([]string, error) { +func newKindsFromLabels(block *hclsyntax.Block, addWildcard bool) ([]string, error) { var allKinds []string for _, kinds := range block.Labels { all := strings.Split(kinds, errorHandlerLabelSep) @@ -133,7 +133,7 @@ func newKindsFromLabels(block *hclsyntax.Block) ([]string, error) { } allKinds = append(allKinds, all...) } - if len(allKinds) == 0 { + if addWildcard && len(allKinds) == 0 { allKinds = append(allKinds, errors.Wildcard) } return allKinds, nil diff --git a/config/configload/error_handler_test.go b/config/configload/error_handler_test.go index 962016ebe..318789eb8 100644 --- a/config/configload/error_handler_test.go +++ b/config/configload/error_handler_test.go @@ -26,7 +26,7 @@ func TestErrorHandler_newKindsFromLabels(t *testing.T) { }, } - _, err := newKindsFromLabels(b) + _, err := newKindsFromLabels(b, true) exp := `message:":123,321-323: empty error_handler label; ", synopsis:"configuration error"` if got := fmt.Sprintf("%#v", err); !strings.Contains(got, exp) { diff --git a/config/configload/load.go b/config/configload/load.go index aa3efdf33..655e2c931 100644 --- a/config/configload/load.go +++ b/config/configload/load.go @@ -11,6 +11,7 @@ import ( "github.com/hashicorp/hcl/v2/gohcl" "github.com/hashicorp/hcl/v2/hclparse" "github.com/hashicorp/hcl/v2/hclsyntax" + "github.com/sirupsen/logrus" "github.com/zclconf/go-cty/cty" "github.com/avenga/couper/config" @@ -139,7 +140,9 @@ func parseFiles(files configfile.Files) ([]*hclsyntax.Body, [][]byte, error) { return parsedBodies, srcBytes, nil } -func bodiesToConfig(parsedBodies []*hclsyntax.Body, srcBytes [][]byte, env string) (*config.Couper, error) { +func bodiesToConfig(parsedBodies []*hclsyntax.Body, srcBytes [][]byte, env string, logger *logrus.Entry) (*config.Couper, error) { + deprecate(parsedBodies, logger) + defaultsBlock, err := mergeDefaults(parsedBodies) if err != nil { return nil, err @@ -196,7 +199,7 @@ func bodiesToConfig(parsedBodies []*hclsyntax.Body, srcBytes [][]byte, env strin return conf, nil } -func LoadFiles(filesList []string, env string) (*config.Couper, error) { +func LoadFiles(filesList []string, env string, logger *logrus.Entry) (*config.Couper, error) { configFiles, err := configfile.NewFiles(filesList) if err != nil { return nil, err @@ -220,7 +223,7 @@ func LoadFiles(filesList []string, env string) (*config.Couper, error) { return nil, diags } if confSettings.Environment != "" { - return LoadFiles(filesList, confSettings.Environment) + return LoadFiles(filesList, confSettings.Environment, logger) } } @@ -228,7 +231,7 @@ func LoadFiles(filesList []string, env string) (*config.Couper, error) { return nil, errorBeforeRetry } - conf, err := bodiesToConfig(parsedBodies, srcBytes, env) + conf, err := bodiesToConfig(parsedBodies, srcBytes, env, logger) if err != nil { return nil, err } @@ -238,7 +241,7 @@ func LoadFiles(filesList []string, env string) (*config.Couper, error) { } func LoadFile(file, env string) (*config.Couper, error) { - return LoadFiles([]string{file}, env) + return LoadFiles([]string{file}, env, nil) } type testContent struct { @@ -262,7 +265,7 @@ func loadTestContents(tcs []testContent) (*config.Couper, error) { srcs = append(srcs, tc.src) } - return bodiesToConfig(parsedBodies, srcs, "") + return bodiesToConfig(parsedBodies, srcs, "", nil) } func LoadBytes(src []byte, filename string) (*config.Couper, error) { @@ -279,7 +282,7 @@ func LoadBytesEnv(src []byte, filename, env string) (*config.Couper, error) { return nil, err } - return bodiesToConfig([]*hclsyntax.Body{hclBody}, [][]byte{src}, env) + return bodiesToConfig([]*hclsyntax.Body{hclBody}, [][]byte{src}, env, nil) } func LoadConfig(body *hclsyntax.Body) (*config.Couper, error) { diff --git a/config/configload/validate.go b/config/configload/validate.go index 7ff975373..8444b4c7a 100644 --- a/config/configload/validate.go +++ b/config/configload/validate.go @@ -315,10 +315,10 @@ func validMethods(methods []string, attr *hclsyntax.Attribute) error { // validateBackendTLS determines irrational backend certificate configuration. func validateBackendTLS(block *hclsyntax.Block) error { - validateCertAttr, _ := block.Body.Attributes["disable_certificate_validation"] + validateCertAttr := block.Body.Attributes["disable_certificate_validation"] if tlsBlocks := bodySyntax.BlocksOfType(block.Body, "tls"); validateCertAttr != nil && len(tlsBlocks) > 0 { var attrName string - if caCert, _ := tlsBlocks[0].Body.Attributes["server_ca_certificate"]; caCert != nil { + if caCert := tlsBlocks[0].Body.Attributes["server_ca_certificate"]; caCert != nil { attrName = caCert.Name } else if caCertFile := tlsBlocks[0].Body.Attributes["server_ca_certificate_file"]; caCertFile != nil { attrName = caCertFile.Name diff --git a/eval/context.go b/eval/context.go index 09585bdd7..6e48d7368 100644 --- a/eval/context.go +++ b/eval/context.go @@ -584,9 +584,11 @@ func newVariable(ctx context.Context, cookies []*http.Cookie, headers http.Heade gp, _ := ctx.Value(request.GrantedPermissions).([]string) if len(gp) > 0 { ctxAcMap[grantedPermissions] = seetie.GoToValue(gp) + ctxAcMap["beta_"+grantedPermissions] = seetie.GoToValue(gp) } if rp, permissionSet := ctx.Value(request.RequiredPermission).(string); permissionSet { ctxAcMap[requiredPermission] = seetie.GoToValue(rp) + ctxAcMap["beta_"+requiredPermission] = seetie.GoToValue(rp) } var ctxAcMapValue cty.Value if len(ctxAcMap) > 0 { diff --git a/main.go b/main.go index 269867076..52ffe0c32 100644 --- a/main.go +++ b/main.go @@ -139,7 +139,7 @@ func realmain(ctx context.Context, arguments []string) int { return 0 } - confFile, err := configload.LoadFiles(filesList.paths, flags.Environment) + confFile, err := configload.LoadFiles(filesList.paths, flags.Environment, log) if err != nil { log.WithError(err).Error() return 1 @@ -223,7 +223,7 @@ func realmain(ctx context.Context, arguments []string) int { errRetries = 0 // reset logger.Info("reloading couper configuration") - cf, reloadErr := configload.LoadFiles(filesList.paths, flags.Environment) + cf, reloadErr := configload.LoadFiles(filesList.paths, flags.Environment, logger) if reloadErr != nil { logger.WithError(reloadErr).Error("reload failed") time.Sleep(flags.FileWatchRetryDelay) @@ -249,7 +249,7 @@ func realmain(ctx context.Context, arguments []string) int { } // Create new config with non-canceled context. - confFile, reloadErr = configload.LoadFiles(filesList.paths, flags.Environment) + confFile, reloadErr = configload.LoadFiles(filesList.paths, flags.Environment, logger) if reloadErr != nil { logger.WithError(reloadErr).Error("reload failed") time.Sleep(flags.FileWatchRetryDelay) diff --git a/server/http_integration_test.go b/server/http_integration_test.go index 00618c03b..b3a35553b 100644 --- a/server/http_integration_test.go +++ b/server/http_integration_test.go @@ -92,7 +92,7 @@ func newCouper(file string, helper *test.Helper) (func(), *logrustest.Hook) { } func newCouperMultiFiles(file, dir string, helper *test.Helper) (func(), *logrustest.Hook) { - couperConfig, err := configload.LoadFiles([]string{file, dir}, "test") + couperConfig, err := configload.LoadFiles([]string{file, dir}, "test", nil) helper.Must(err) return newCouperWithConfig(couperConfig, helper)