Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cloud: Allows object tag schema for selecting key/value tagged workspaces #35907

Merged
merged 4 commits into from
Oct 29, 2024

Conversation

brandonc
Copy link
Contributor

@brandonc brandonc commented Oct 26, 2024

When key-value tags are enabled and used in the workspace, users may define the tags attribute as a map of strings in the cloud block in order to more precisely match workspaces using those tags.

Target Release

1.10.x

Draft CHANGELOG entry

ENHANCEMENTS

  • cloud block workspace tag attribute can now be defined as either a set(string) (matching key only) or a map(string) (matching keys and values together)
Example output: init cloud block with kv tags
$ ./terraform init
Initializing HCP Terraform...

No workspaces found.

  There are no workspaces with the configured tags (cc=101, dept=billing)
  in your HCP Terraform organization. To finish initializing, Terraform needs at
  least one workspace available.

  Terraform can create a properly tagged workspace for you now. Please enter a
  name to create a new HCP Terraform workspace.

  Enter a value:

When key-value tags are enabled and used in the workspace, users may
define the tags attribute as a map of strings in the cloud block in order
to more precicely match workspaces using those tags.
@brandonc brandonc force-pushed the brandonc/resource_tags_cloud_block branch from 05d8a3e to e87335a Compare October 26, 2024 14:04
@brandonc brandonc marked this pull request as ready for review October 26, 2024 14:13
@brandonc brandonc requested review from a team as code owners October 26, 2024 14:13
Copy link
Contributor

@Maed223 Maed223 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From smoke testing I'm seeing a couple problems

In using the old single value tags:

Cloud Block:

cloud {
    hostname     = "app.staging.terraform.io"
    organization = "markdecrane"
    workspaces {
      tags = ["app", "test"]
    }
  }

Error:
Screenshot 2024-10-28 at 5 19 30 PM

Besides the fact that this is occuring, it seems odd to me that the errors Error: failed to create backend alias to target "". The hostname is not in the correct format. and Error: Invalid workspaces configuration are given when an incorrect type for tags is given. Saw nearly the same diags on the console when giving tags the value of a single string.

Screenshot 2024-10-28 at 5 40 43 PM

Nil panics when the value of of tag is not a string

I'm also seeing a need for some more error handling as I get nil panics when the value of of tag is not a string:

Screenshot 2024-10-28 at 5 55 15 PM

Weirdly not a problem when the key itself is not a string

workspaces {
      tags = {
        1 = "foo"
        2 = "bar"
      }
    }
Screenshot 2024-10-28 at 5 56 32 PM

Looks as though it's somehow treated as a string considering it doesn't break things at any point in the workflow, despite this error message I came across when specifying the key as a set, dictating that the key must be a string:

Screenshot 2024-10-28 at 6 02 29 PM

@brandonc
Copy link
Contributor Author

@Maed223 thanks for the thorough testing! There was an interesting mismatch between the existing tests and the actual type that happens when you write config (Set vs Tuple) which broke the legacy use cases! Glad we caught that.

I also detected and fixed the case when element types were not strings. In the case of keys not being strings, that's built into the syntax of the language (all object/map keys are strings). For instance, you can define an object like { hello = "world" }

Copy link
Contributor

@Maed223 Maed223 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple more behavioral oddities I detected, one of which might be on the atlas API side of things but I'll note here.

Single-Value-Tag/ KV-Tag matching

Saw that choosing a workspace when single value tags (in config) are matched to the key values of KV tags (on the workspace) there's a duplication of the tags– in that the KV tags that are being matched to are persisted and additional single value legacy tags are created on the workspace.

For example:

workspaces {
  tags = ["app", "test"]
}
Screenshot 2024-10-29 at 11 31 14 AM Screenshot 2024-10-29 at 11 31 29 AM

Also saw that the inverse of this occurs as well. As in KV tags in config matching to single value tags on a workspace persists the single value tags, while adding the new KV tags.

Wondering what the intention is for the behavior here. I can see the value in adding the KV tags when the workspace only has the single value tags considering the KV tags hold more info (but should the original legacy tags persist?), but less so for the inverse case. No strong opinion here, just saw that this aspect wasn't specifically discussed in the RFC.

Extraneous error message when giving incorrect type to value of KV tag

Less than before, but still seeing the error related to the hostname when giving a non-string value as a KV tag's value. Not breaking, but feels less than ideal.
Screenshot 2024-10-29 at 11 36 43 AM

@brandonc
Copy link
Contributor Author

@Maed223 These are interesting questions and yes, a mismatch between how workspaces are found vs how "workspaceTagsRequireUpdate" detects. I do think I want to refine the behavior, but since the beta is getting cut tomorrow, do you mind if we merge it in this state and refine it during the beta period?

I'll look into the alias error as well, but I think that is unrelated to the change.

@brandonc brandonc merged commit 10530bb into main Oct 29, 2024
6 checks passed
@brandonc brandonc deleted the brandonc/resource_tags_cloud_block branch October 29, 2024 20:19
Copy link
Contributor

Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch.

Copy link
Contributor

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants