Repository: avenga/couper
Pull request #546 (2022-07-28 11:28:23)

Environment startup failure
Fail if environment blocks are used without COUPER_ENVIRONMENT being set.

---
<details>
    <summary>Reviewer checklist</summary>
    <ul>
        <li>Read PR description: a summary about the changes is required</li>
        <li>Changelog updated</li>
        <li>Documentation: docs/{Reference, Cli, ...}, Docker and cli help/usage</li>
        <li>Pulled branch, manually tested</li>
        <li>Verified requirements are met</li>
        <li>Reviewed the code</li>
        <li>Reviewed the related tests</li>
    </ul>
</details>



Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 33f6f132..8a4b1984 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,9 @@ Unreleased changes are available as `avenga/couper:edge` container.
 * **Added**
   * [`environment` block](https://docs.couper.io/configuration/block/environment), [setting](https://docs.couper.io/configuration/block/settings) and [`couper.environment` variable](https://docs.couper.io/configuration/variables#couper) ([#521](https://github.com/avenga/couper/pull/521), ([#534](https://github.com/avenga/couper/pull/534), [#545](https://github.com/avenga/couper/pull/545))
 
+* **Changed**
+ * Starting will now fail if `environment` blocks are used without `COUPER_ENVIRONMENT` being set ([#546](https://github.com/avenga/couper/pull/546))
+
 * **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))
diff --git a/config/configload/environment.go b/config/configload/environment.go
index f06d2a22..a249f15b 100644
--- a/config/configload/environment.go
+++ b/config/configload/environment.go
@@ -1,57 +1,68 @@
 package configload
 
 import (
+	"fmt"
 	"github.com/hashicorp/hcl/v2/hclsyntax"
 )
 
 func preprocessEnvironmentBlocks(bodies []*hclsyntax.Body, env string) error {
+	found := false
 	for _, body := range bodies {
-		if err := preprocessBody(body, env); err != nil {
+		f, err := preprocessBody(body, env)
+		if err != nil {
 			return err
 		}
+		found = found || f
+	}
+
+	if found && env == "" {
+		return fmt.Errorf(`"environment" blocks found, but "COUPER_ENVIRONMENT" setting is missing`)
 	}
 
 	return nil
 }
 
-func preprocessBody(parent *hclsyntax.Body, env string) error {
+func preprocessBody(body *hclsyntax.Body, env string) (bool, error) {
 	var blocks []*hclsyntax.Block
+	found := false
 
-	for _, block := range parent.Blocks {
+	for _, block := range body.Blocks {
 		if block.Type != environment {
 			blocks = append(blocks, block)
-
 			continue
 		}
 
+		found = true
+
 		if len(block.Labels) == 0 {
 			defRange := block.DefRange()
-
-			return newDiagErr(&defRange, "Missing label(s) for 'environment' block")
+			return true, newDiagErr(&defRange, "Missing label(s) for 'environment' block")
 		}
 
 		for _, label := range block.Labels {
 			if err := validLabel(label, getRange(block.Body)); err != nil {
-				return err
+				return true, err
 			}
 
 			if label == env {
 				blocks = append(blocks, block.Body.Blocks...)
 
 				for name, attr := range block.Body.Attributes {
-					parent.Attributes[name] = attr
+					body.Attributes[name] = attr
 				}
 			}
 		}
 	}
 
 	for _, block := range blocks {
-		if err := preprocessBody(block.Body, env); err != nil {
-			return err
+		foundInChildren, err := preprocessBody(block.Body, env)
+		if err != nil {
+			return found || foundInChildren, err
 		}
+		found = found || foundInChildren
 	}
 
-	parent.Blocks = blocks
+	body.Blocks = blocks
 
-	return nil
+	return found, nil
 }
diff --git a/config/configload/load.go b/config/configload/load.go
index 5ac09ed6..635985d4 100644
--- a/config/configload/load.go
+++ b/config/configload/load.go
@@ -141,9 +141,7 @@ func LoadFiles(filesList []string, env string) (*config.Couper, error) {
 		return nil, fmt.Errorf("missing configuration files")
 	}
 
-	if err := preprocessEnvironmentBlocks(parsedBodies, env); err != nil {
-		return nil, err
-	}
+	errorBeforeRetry := preprocessEnvironmentBlocks(parsedBodies, env)
 
 	if env == "" {
 		settingsBlock := mergeSettings(parsedBodies)
@@ -156,6 +154,10 @@ func LoadFiles(filesList []string, env string) (*config.Couper, error) {
 		}
 	}
 
+	if errorBeforeRetry != nil {
+		return nil, errorBeforeRetry
+	}
+
 	defaultsBlock, err := mergeDefaults(parsedBodies)
 	if err != nil {
 		return nil, err
diff --git a/docs/website/content/2.configuration/4.block/environment.md b/docs/website/content/2.configuration/4.block/environment.md
index 9bc49643..086cd82e 100644
--- a/docs/website/content/2.configuration/4.block/environment.md
+++ b/docs/website/content/2.configuration/4.block/environment.md
@@ -12,6 +12,9 @@ block does not match the set [`COUPER_ENVIRONMENT`](../command-line#global-optio
 removes this block and its content. Otherwise, the content of the block is added
 to the configuration.
 
+Starting Couper with a configuration containing `environment` blocks will fail, if `COUPER_ENVIRONMENT`
+is missing or empty. A default value can be set using the [`settings` block](settings).
+
 ## Example
 
 Considering the following configuration with the `COUPER_ENVIRONMENT` value set to `prod`


Test patch:
diff --git a/config/configload/load_test.go b/config/configload/load_test.go
index b2fc9e82..1cd3df7b 100644
--- a/config/configload/load_test.go
+++ b/config/configload/load_test.go
@@ -3,6 +3,7 @@ package configload_test
 import (
 	"context"
 	"fmt"
+	"os"
 	"strings"
 	"testing"
 
@@ -249,3 +250,98 @@ func TestEndpointPaths(t *testing.T) {
 		})
 	}
 }
+
+func TestEnvironmentBlocksWithoutEnvironment(t *testing.T) {
+	tests := []struct {
+		name string
+		hcl  string
+		env  string
+		want string
+	}{
+		{
+			"no environment block, no setting",
+			`
+			definitions {}
+			`,
+			"",
+			"configuration error: missing 'server' block",
+		},
+
+		{
+			"environment block, but no setting",
+			`
+			environment "foo" {}
+			server {}
+			`,
+			"",
+			`"environment" blocks found, but "COUPER_ENVIRONMENT" setting is missing`,
+		},
+		{
+			"environment block & setting",
+			`
+			environment "foo" {
+			  server {}
+			}
+			`,
+			"bar",
+			"configuration error: missing 'server' block",
+		},
+		{
+			"environment block & setting",
+			`
+			server {
+			  environment "foo" {
+			    endpoint "/" {}
+			  }
+			}
+			`,
+			"foo",
+			"missing 'default' proxy or request block, or a response definition",
+		},
+		{
+			"environment block & default setting",
+			`
+			environment "foo" {
+			  server {}
+			}
+			settings {
+				environment = "bar"
+			}
+			`,
+			"",
+			"configuration error: missing 'server' block",
+		},
+	}
+
+	helper := test.New(t)
+
+	file, err := os.CreateTemp("", "tmpfile-")
+	helper.Must(err)
+	defer file.Close()
+	defer os.Remove(file.Name())
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(subT *testing.T) {
+			config := []byte(tt.hcl)
+			err := os.Truncate(file.Name(), 0)
+			helper.Must(err)
+			_, err = file.Seek(0, 0)
+			helper.Must(err)
+			_, err = file.Write(config)
+
+			_, err = configload.LoadFile(file.Name(), tt.env)
+			if err == nil && tt.want != "" {
+				subT.Errorf("Missing expected error:\nWant:\t%q\nGot:\tnil", tt.want)
+				return
+			}
+
+			if err != nil {
+				message := err.Error()
+				if !strings.Contains(message, tt.want) {
+					subT.Errorf("Unexpected error message:\nWant:\t%q\nGot:\t%q", tt.want, message)
+					return
+				}
+			}
+		})
+	}
+}
diff --git a/server/http_integration_test.go b/server/http_integration_test.go
index 18b6589e..cc2d7ce4 100644
--- a/server/http_integration_test.go
+++ b/server/http_integration_test.go
@@ -5461,7 +5461,6 @@ func TestEnvironmentSetting(t *testing.T) {
 	tests := []struct {
 		env string
 	}{
-		{""},
 		{"foo"},
 		{"bar"},
 	}