From 23d69e23513f7d4b21b1c73c64bbf052de0b9e24 Mon Sep 17 00:00:00 2001 From: Ronny Orot Date: Thu, 22 Aug 2024 10:55:50 +0300 Subject: [PATCH] Cloud Backend - Fix logic that forces TF_WORKSPACE to be equal to a tag name (#1930) Signed-off-by: Ronny Orot --- internal/cloud/backend.go | 23 +++++-------- internal/cloud/backend_test.go | 50 +++++++++++++++++++++-------- internal/cloud/errors.go | 16 +++++---- website/docs/cli/cloud/settings.mdx | 2 +- 4 files changed, 54 insertions(+), 37 deletions(-) diff --git a/internal/cloud/backend.go b/internal/cloud/backend.go index 0f356fb21f..b00202e805 100644 --- a/internal/cloud/backend.go +++ b/internal/cloud/backend.go @@ -465,13 +465,15 @@ func (b *Cloud) setConfigurationFields(obj cty.Value) tfdiags.Diagnostics { } func reconcileWorkspaceMappingEnvVars(w *WorkspaceMapping) tfdiags.Diagnostic { - // See: https://github.com/opentofu/opentofu/issues/814 - if v := os.Getenv("TF_WORKSPACE"); v != "" && w.Name == "" { - if len(w.Tags) > 0 && !workspaceInTags(w.Tags, v) { - return invalidWorkspaceConfigMisconfigurationEnvVar + if v := os.Getenv("TF_WORKSPACE"); v != "" { + if w.Name != "" && w.Name != v { + return invalidWorkspaceConfigInconsistentNameAndEnvVar() + } + + // If we don't have workspaces name or tags set in config, we can get the name from the TF_WORKSPACE env var + if w.Strategy() == WorkspaceNoneStrategy { + w.Name = v } - w.Name = v - w.Tags = nil } if v := os.Getenv("TF_CLOUD_PROJECT"); v != "" && w.Project == "" { @@ -481,15 +483,6 @@ func reconcileWorkspaceMappingEnvVars(w *WorkspaceMapping) tfdiags.Diagnostic { return nil } -func workspaceInTags(tags []string, workspace string) bool { - for _, tag := range tags { - if tag == workspace { - return true - } - } - return false -} - // discover the TFC/E API service URL and version constraints. func (b *Cloud) discover() (*url.URL, error) { hostname, err := svchost.ForComparison(b.hostname) diff --git a/internal/cloud/backend_test.go b/internal/cloud/backend_test.go index 6ad700c433..9a053a909a 100644 --- a/internal/cloud/backend_test.go +++ b/internal/cloud/backend_test.go @@ -351,6 +351,7 @@ func TestCloud_config(t *testing.T) { config cty.Value confErr string valErr string + envVars map[string]string }{ "with_a_non_tfe_host": { config: cty.ObjectVal(map[string]cty.Value{ @@ -440,10 +441,33 @@ func TestCloud_config(t *testing.T) { "null config": { config: cty.NullVal(cty.EmptyObject), }, + "with_tags_and_TF_WORKSPACE_env_var_not_matching_tags": { //TODO: once we have proper e2e backend testing we should also add the opposite test - with_tags_and_TF_WORKSPACE_env_var_matching_tags + config: cty.ObjectVal(map[string]cty.Value{ + "hostname": cty.NullVal(cty.String), + "organization": cty.StringVal("opentofu"), + "token": cty.NullVal(cty.String), + "workspaces": cty.ObjectVal(map[string]cty.Value{ + "tags": cty.SetVal( + []cty.Value{ + cty.StringVal("billing"), + }, + ), + "project": cty.NullVal(cty.String), + }), + }), + envVars: map[string]string{ + "TF_WORKSPACE": "my-workspace", + }, + confErr: `OpenTofu failed to find workspace my-workspace with the tags specified in your configuration`, + }, } for name, tc := range cases { t.Run(name, func(t *testing.T) { + for k, v := range tc.envVars { + t.Setenv(k, v) + } + b, cleanup := testUnconfiguredBackend(t) t.Cleanup(cleanup) @@ -660,7 +684,7 @@ func TestCloud_setConfigurationFieldsHappyPath(t *testing.T) { }, expectedForceLocal: true, }, - "with hostname and workspace tags set, and tags overwritten by TF_WORKSPACE": { + "with hostname and workspace tags set, then tags should not be overwritten by TF_WORKSPACE": { // see: https://github.com/opentofu/opentofu/issues/814 obj: cty.ObjectVal(map[string]cty.Value{ "organization": cty.NullVal(cty.String), @@ -675,25 +699,24 @@ func TestCloud_setConfigurationFieldsHappyPath(t *testing.T) { "TF_WORKSPACE": "foo", }, expectedHostname: "opentofu.org", - expectedWorkspaceName: "foo", - expectedWorkspaceTags: nil, + expectedWorkspaceName: "", + expectedWorkspaceTags: map[string]struct{}{"foo": {}, "bar": {}}, }, - "with hostname and workspace name set, and TF_WORKSPACE specified": { + "with hostname and workspace name set, and workspace name the same as provided TF_WORKSPACE": { obj: cty.ObjectVal(map[string]cty.Value{ "organization": cty.NullVal(cty.String), "hostname": cty.StringVal("opentofu.org"), "workspaces": cty.ObjectVal(map[string]cty.Value{ - "name": cty.StringVal("old"), + "name": cty.StringVal("my-workspace"), "tags": cty.NullVal(cty.Set(cty.String)), "project": cty.NullVal(cty.String), }), }), envVars: map[string]string{ - "TF_WORKSPACE": "new", + "TF_WORKSPACE": "my-workspace", }, expectedHostname: "opentofu.org", - expectedWorkspaceName: "old", - expectedWorkspaceTags: nil, + expectedWorkspaceName: "my-workspace", }, "with hostname and project set, and project overwritten by TF_CLOUD_PROJECT": { obj: cty.ObjectVal(map[string]cty.Value{ @@ -858,22 +881,21 @@ func TestCloud_setConfigurationFieldsUnhappyPath(t *testing.T) { wantSummary: "Hostname is required for the cloud backend", wantDetail: `OpenTofu does not provide a default "hostname" attribute, so it must be set to the hostname of the cloud backend.`, }, - "with hostname and workspace tags set, and tags overwritten by TF_WORKSPACE": { - // see: https://github.com/opentofu/opentofu/issues/814 + "with hostname and workspace name set, and workspace name is not the same as provided TF_WORKSPACE": { obj: cty.ObjectVal(map[string]cty.Value{ "organization": cty.NullVal(cty.String), "hostname": cty.StringVal("opentofu.org"), "workspaces": cty.ObjectVal(map[string]cty.Value{ - "name": cty.NullVal(cty.String), - "tags": cty.SetVal([]cty.Value{cty.StringVal("foo"), cty.StringVal("bar")}), + "name": cty.StringVal("my-workspace"), + "tags": cty.NullVal(cty.Set(cty.String)), "project": cty.NullVal(cty.String), }), }), envVars: map[string]string{ "TF_WORKSPACE": "qux", }, - wantSummary: invalidWorkspaceConfigMisconfigurationEnvVar.Description().Summary, - wantDetail: invalidWorkspaceConfigMisconfigurationEnvVar.Description().Detail, + wantSummary: invalidWorkspaceConfigInconsistentNameAndEnvVar().Description().Summary, + wantDetail: invalidWorkspaceConfigInconsistentNameAndEnvVar().Description().Detail, }, } diff --git a/internal/cloud/errors.go b/internal/cloud/errors.go index 43d8a82609..20f19fda97 100644 --- a/internal/cloud/errors.go +++ b/internal/cloud/errors.go @@ -40,13 +40,6 @@ var ( fmt.Sprintf("Only one of workspace \"tags\" or \"name\" is allowed.\n\n%s", workspaceConfigurationHelp), cty.Path{cty.GetAttrStep{Name: "workspaces"}}, ) - - invalidWorkspaceConfigMisconfigurationEnvVar = tfdiags.AttributeValue( - tfdiags.Error, - "Invalid workspaces configuration", - fmt.Sprintf("The workspace defined using the environment variable \"TF_WORKSPACE\" does not belong to \"tags\".\n\n%s", workspaceConfigurationHelp), - cty.Path{cty.GetAttrStep{Name: "workspaces"}}, - ) ) const ignoreRemoteVersionHelp = "If you're sure you want to upgrade the state, you can force OpenTofu to continue using the -ignore-remote-version flag. This may result in an unusable workspace." @@ -70,3 +63,12 @@ func incompatibleWorkspaceTerraformVersion(message string, ignoreVersionConflict description := strings.TrimSpace(fmt.Sprintf("%s\n\n%s", message, suggestion)) return tfdiags.Sourceless(severity, "Incompatible TF version", description) } + +func invalidWorkspaceConfigInconsistentNameAndEnvVar() tfdiags.Diagnostic { + return tfdiags.AttributeValue( + tfdiags.Error, + "Invalid workspaces configuration", + fmt.Sprintf("The workspace defined using the environment variable \"TF_WORKSPACE\" is not consistent with the workspace \"name\" in the configuration.\n\n%s", workspaceConfigurationHelp), + cty.Path{cty.GetAttrStep{Name: "workspaces"}}, + ) +} diff --git a/website/docs/cli/cloud/settings.mdx b/website/docs/cli/cloud/settings.mdx index 6b3fa2fecd..b7d23f6060 100644 --- a/website/docs/cli/cloud/settings.mdx +++ b/website/docs/cli/cloud/settings.mdx @@ -100,7 +100,7 @@ Use the following environment variables to configure the `cloud` block: - `TF_CLOUD_PROJECT` - The name of a cloud backend project. OpenTofu reads this when `workspaces.project` is omitted from the `cloud` block. If both are specified, the cloud block configuration takes precedence. -- `TF_WORKSPACE` - The name of a single cloud backend workspace. OpenTofu reads this when `workspaces` is omitted from the `cloud` block. The cloud backend will not create a new workspace from this variable; the workspace must exist in the specified organization. You can set `TF_WORKSPACE` if the `cloud` block uses tags. However, the value of `TF_WORKSPACE` must be included in the set of tags. This variable also selects the workspace in your local environment. Refer to [TF_WORKSPACE](../config/environment-variables.mdx#tf_workspace) for details. +- `TF_WORKSPACE` - The name of a single cloud backend workspace. OpenTofu reads this when `workspaces` is omitted from the `cloud` block. The cloud backend will not create a new workspace from this variable; the workspace must exist in the specified organization. You can set `TF_WORKSPACE` if the `cloud` block uses tags. However, the selected `TF_WORKSPACE` must have a set of tags that match the tags in the `cloud` block. This variable also selects the workspace in your local environment. Refer to [TF_WORKSPACE](../config/environment-variables.mdx#tf_workspace) for details. ## Excluding Files from Upload with .terraformignore