Pre merge validation (#563)

* call validateBody() for each parsed HCL body; check label for definition child blocks; check label uniqueness for backend labels * move backend label validity check * move backend label anonymous check * AC missing/empty label check * AC reserved label check * check label uniqueness for AC labels * check endpoint path pattern uniqueness * repeat uniqueness checks on merged config * split and rename tests * changelog entry * white-space-trim backend labels, too * use label range for invalid environment label * don't export struct and load function currently only used by tests * use nil context * extract code to function * use separate variables * extract code to function * check jwt signing profiles Co-authored-by: Alex Schneider <alex.schneider@avenga.com>
coupergateway · Aug 31, 2022 · 7bc88d7 · 7bc88d7
1 parent 62de587
commit 7bc88d7
Show file tree

Hide file tree

Showing 14 changed files with 912 additions and 326 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -16,6 +16,7 @@ Unreleased changes are available as `avenga/couper:edge` container.
 * **Fixed**
   * Disallow empty path parameters ([#526](https://github.com/avenga/couper/pull/526))
   * Basic Auth client authentication with OAuth2 (client ID and secret must be URL encoded) ([#537](https://github.com/avenga/couper/pull/537))
+  * Config validation, e.g. label-uniqueness checks ([#563](https://github.com/avenga/couper/pull/563))
 
 * **Removed**
   * Endpoint path normalization to better match OpenAPI behavior ([#526](https://github.com/avenga/couper/pull/526))

diff --git a/config/configload/backend.go b/config/configload/backend.go
@@ -100,10 +100,7 @@ func PrepareBackend(helper *helper, attrName, attrValue string, block config.Inl
 			backendBody = hclbody.MergeBodies(defaultBackend, backendBody)
 		}
 
-		backendBody, err = newBodyWithName(anonLabel, backendBody)
-		if err != nil {
-			return nil, err
-		}
+		backendBody = newBodyWithName(anonLabel, backendBody)
 	}
 
 	// watch out for oauth blocks and nested backend definitions
@@ -255,15 +252,11 @@ func newBlock(blockType string, content hcl.Body) hcl.Body {
 	})
 }
 
-func newBodyWithName(nameValue string, config hcl.Body) (hcl.Body, error) {
-	if err := validLabel(nameValue, getRange(config)); err != nil {
-		return nil, err
-	}
-
+func newBodyWithName(nameValue string, config hcl.Body) hcl.Body {
 	return hclbody.MergeBodies(
 		config,
 		hclbody.New(hclbody.NewContentWithAttrName("name", nameValue)),
-	), nil
+	)
 }
 
 func newAnonLabel(body hcl.Body, labelRange *hcl.Range) string {

diff --git a/config/configload/environment.go b/config/configload/environment.go
@@ -39,8 +39,8 @@ func preprocessBody(body *hclsyntax.Body, env string) (bool, error) {
 			return true, newDiagErr(&defRange, "Missing label(s) for 'environment' block")
 		}
 
-		for _, label := range block.Labels {
-			if err := validLabel(label, getRange(block.Body)); err != nil {
+		for i, label := range block.Labels {
+			if err := validLabel(label, &block.LabelRanges[i]); err != nil {
 				return true, err
 			}
 

diff --git a/config/configload/helper.go b/config/configload/helper.go
@@ -51,24 +51,12 @@ func newHelper(body hcl.Body, src [][]byte, environment string) (*helper, error)
 	}, nil
 }
 
-func (h *helper) addBackend(block *hcl.Block) error {
+func (h *helper) addBackend(block *hcl.Block) {
 	name := block.Labels[0]
 
-	if _, ok := h.defsBackends[name]; ok {
-		return newDiagErr(&block.LabelRanges[0],
-			fmt.Sprintf("duplicate backend name: %q", name))
-	} else if strings.HasPrefix(name, "anonymous_") {
-		return newDiagErr(&block.LabelRanges[0],
-			fmt.Sprintf("backend name must not start with 'anonymous_': %q", name))
-	}
-
-	backendBody, err := newBodyWithName(name, block.Body)
-	if err != nil {
-		return err
-	}
+	backendBody := newBodyWithName(name, block.Body)
 
 	h.defsBackends[name] = backendBody
-	return nil
 }
 
 func (h *helper) configureDefinedBackends() error {

diff --git a/config/configload/load.go b/config/configload/load.go
@@ -128,38 +128,7 @@ func parseFiles(files configfile.Files) ([]*hclsyntax.Body, [][]byte, error) {
 	return parsedBodies, srcBytes, nil
 }
 
-func LoadFiles(filesList []string, env string) (*config.Couper, error) {
-	configFiles, err := configfile.NewFiles(filesList)
-	if err != nil {
-		return nil, err
-	}
-
-	parsedBodies, srcBytes, err := parseFiles(configFiles)
-	if err != nil {
-		return nil, err
-	}
-
-	if len(srcBytes) == 0 {
-		return nil, fmt.Errorf("missing configuration files")
-	}
-
-	errorBeforeRetry := preprocessEnvironmentBlocks(parsedBodies, env)
-
-	if env == "" {
-		settingsBlock := mergeSettings(parsedBodies)
-		settings := &config.Settings{}
-		if diags := gohcl.DecodeBody(settingsBlock.Body, nil, settings); diags.HasErrors() {
-			return nil, diags
-		}
-		if settings.Environment != "" {
-			return LoadFiles(filesList, settings.Environment)
-		}
-	}
-
-	if errorBeforeRetry != nil {
-		return nil, errorBeforeRetry
-	}
-
+func bodiesToConfig(parsedBodies []*hclsyntax.Body, srcBytes [][]byte, env string) (*config.Couper, error) {
 	defaultsBlock, err := mergeDefaults(parsedBodies)
 	if err != nil {
 		return nil, err
@@ -177,6 +146,10 @@ func LoadFiles(filesList []string, env string) (*config.Couper, error) {
 		if err = absolutizePaths(body); err != nil {
 			return nil, err
 		}
+
+		if err = validateBody(body, false); err != nil {
+			return nil, err
+		}
 	}
 
 	settingsBlock := mergeSettings(parsedBodies)
@@ -200,11 +173,54 @@ func LoadFiles(filesList []string, env string) (*config.Couper, error) {
 		Blocks: configBlocks,
 	}
 
+	if err = validateBody(configBody, true); err != nil {
+		return nil, err
+	}
+
 	conf, err := LoadConfig(configBody, srcBytes, env)
 	if err != nil {
 		return nil, err
 	}
 
+	return conf, nil
+}
+
+func LoadFiles(filesList []string, env string) (*config.Couper, error) {
+	configFiles, err := configfile.NewFiles(filesList)
+	if err != nil {
+		return nil, err
+	}
+
+	parsedBodies, srcBytes, err := parseFiles(configFiles)
+	if err != nil {
+		return nil, err
+	}
+
+	if len(srcBytes) == 0 {
+		return nil, fmt.Errorf("missing configuration files")
+	}
+
+	errorBeforeRetry := preprocessEnvironmentBlocks(parsedBodies, env)
+
+	if env == "" {
+		settingsBlock := mergeSettings(parsedBodies)
+		settings := &config.Settings{}
+		if diags := gohcl.DecodeBody(settingsBlock.Body, nil, settings); diags.HasErrors() {
+			return nil, diags
+		}
+		if settings.Environment != "" {
+			return LoadFiles(filesList, settings.Environment)
+		}
+	}
+
+	if errorBeforeRetry != nil {
+		return nil, errorBeforeRetry
+	}
+
+	conf, err := bodiesToConfig(parsedBodies, srcBytes, env)
+	if err != nil {
+		return nil, err
+	}
 	conf.Files = configFiles
 
 	return conf, nil
@@ -214,12 +230,40 @@ func LoadFile(file, env string) (*config.Couper, error) {
 	return LoadFiles([]string{file}, env)
 }
 
+type testContent struct {
+	filename string
+	src      []byte
+}
+
+func loadTestContents(tcs []testContent) (*config.Couper, error) {
+	var (
+		parsedBodies []*hclsyntax.Body
+		srcs         [][]byte
+	)
+
+	for _, tc := range tcs {
+		hclBody, diags := parser.Load(tc.src, tc.filename)
+		if diags.HasErrors() {
+			return nil, diags
+		}
+
+		parsedBodies = append(parsedBodies, hclBody.(*hclsyntax.Body))
+		srcs = append(srcs, tc.src)
+	}
+
+	return bodiesToConfig(parsedBodies, srcs, "")
+}
+
 func LoadBytes(src []byte, filename string) (*config.Couper, error) {
 	hclBody, diags := parser.Load(src, filename)
 	if diags.HasErrors() {
 		return nil, diags
 	}
 
+	if err := validateBody(hclBody, false); err != nil {
+		return nil, err
+	}
+
 	return LoadConfig(hclBody, [][]byte{src}, "")
 }
 
@@ -246,9 +290,7 @@ func LoadConfig(body hcl.Body, src [][]byte, environment string) (*config.Couper
 			// backends first
 			if backendContent != nil {
 				for _, be := range backendContent.Blocks {
-					if err = helper.addBackend(be); err != nil {
-						return nil, err
-					}
+					helper.addBackend(be)
 				}
 
 				if err = helper.configureDefinedBackends(); err != nil {
@@ -316,9 +358,6 @@ func LoadConfig(body hcl.Body, src [][]byte, environment string) (*config.Couper
 
 	jwtSigningConfigs := make(map[string]*lib.JWTSigningConfig)
 	for _, profile := range helper.config.Definitions.JWTSigningProfile {
-		if _, exists := jwtSigningConfigs[profile.Name]; exists {
-			return nil, errors.Configuration.Messagef("jwt_signing_profile block with label %s already defined", profile.Name)
-		}
 		signConf, err := lib.NewJWTSigningConfigFromJWTSigningProfile(profile)
 		if err != nil {
 			return nil, errors.Configuration.Label(profile.Name).With(err)
@@ -331,9 +370,6 @@ func LoadConfig(body hcl.Body, src [][]byte, environment string) (*config.Couper
 			return nil, errors.Configuration.Label(jwt.Name).With(err)
 		}
 		if signConf != nil {
-			if _, exists := jwtSigningConfigs[jwt.Name]; exists {
-				return nil, errors.Configuration.Messagef("jwt_signing_profile or jwt with label %s already defined", jwt.Name)
-			}
 			jwtSigningConfigs[jwt.Name] = signConf
 		}
 	}

diff --git a/config/configload/merge.go b/config/configload/merge.go
@@ -415,10 +415,6 @@ func mergeDefinitions(bodies []*hclsyntax.Body) (*hclsyntax.Block, error) {
 						definitionsBlock[innerBlock.Type] = make(data)
 					}
 
-					if len(innerBlock.Labels) == 0 {
-						return nil, newMergeError(errUniqueLabels, innerBlock)
-					}
-
 					definitionsBlock[innerBlock.Type][innerBlock.Labels[0]] = innerBlock
 
 					// Count the "backend" blocks and "backend" attributes to