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

[provider] Add a default_tags attribute in the Datadog provider config. #2486

Merged
merged 3 commits into from
Aug 7, 2024

Conversation

Amaury-Behague
Copy link
Contributor

@Amaury-Behague Amaury-Behague commented Jul 15, 2024

This PR is a first step towards answering this feature request: #1478

For the moment this config option is experimental and only applies to Datadog monitors.
When default_tags is set at the provider level, resources that do not otherwise define those tags (by key) will inherit them automatically during plan.
If the same tag (e.g. "foo:bar", "foo:not_bar") is defined at provider level and at resource level, the resource tag is used.

@Amaury-Behague Amaury-Behague requested a review from a team July 15, 2024 07:34
@Amaury-Behague Amaury-Behague changed the title Add a default_tags attribute in the Datadog provider config. [datadog_monitor] Add a default_tags attribute in the Datadog provider config. Jul 15, 2024
@Amaury-Behague Amaury-Behague marked this pull request as ready for review July 17, 2024 08:27
@Amaury-Behague Amaury-Behague requested review from a team as code owners July 17, 2024 08:27
drichards-87
drichards-87 previously approved these changes Jul 17, 2024
Copy link
Contributor

@drichards-87 drichards-87 left a comment

Choose a reason for hiding this comment

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

Left a couple of suggestions from Docs and approved the PR.

docs/index.md Outdated
@@ -52,9 +52,17 @@ provider "datadog" {
- `api_key` (String, Sensitive) (Required unless validate is false) Datadog API key. This can also be set via the DD_API_KEY environment variable.
- `api_url` (String) The API URL. This can also be set via the DD_HOST environment variable. Note that this URL must not end with the `/api/` path. For example, `https://api.datadoghq.com/` is a correct value, while `https://api.datadoghq.com/api/` is not. And if you're working with "EU" version of Datadog, use `https://api.datadoghq.eu/`. Other Datadog region examples: `https://api.us5.datadoghq.com/`, `https://api.us3.datadoghq.com/` and `https://api.ddog-gov.com/`. See https://docs.datadoghq.com/getting_started/site/ for all available regions.
- `app_key` (String, Sensitive) (Required unless validate is false) Datadog APP key. This can also be set via the DD_APP_KEY environment variable.
- `default_tags` (Block List, Max: 1) [Experimental - Monitors only] Configuration block with settings to default resource tags across all resources. (see [below for nested schema](#nestedblock--default_tags))
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- `default_tags` (Block List, Max: 1) [Experimental - Monitors only] Configuration block with settings to default resource tags across all resources. (see [below for nested schema](#nestedblock--default_tags))
- `default_tags` (Block List, Max: 1) [Experimental - Monitors only] Configuration block containing settings to apply default resource tags across all resources. (See [below for nested schema](#nestedblock--default_tags).)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated this in the source and ran make docs - the only thing that I could not change is the uppercase on see.

docs/index.md Outdated

Optional:

- `tags` (Map of String) [Experimental - Monitors only] Resource tags to default across all resources
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- `tags` (Map of String) [Experimental - Monitors only] Resource tags to default across all resources
- `tags` (Map of String) [Experimental - Monitors only] Resource tags to be applied by default across all resources.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated this in the source and ran make docs.

Blocks: map[string]schema.Block{
"default_tags": schema.ListNestedBlock{
Validators: []validator.List{
listvalidator.SizeAtMost(1),
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the list can only have size 1 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because we only want tags in there, nothing else - the actual list of tags can have more that one tag.
It's a bit strange (I could have gone for default_tags{} instead of default_tags{tags{}}) but it's the way it's done for the AWS provider, so it's how the feature was requested and probably how the users expect this to work.

For the moment this config option is experimental and only applies
to Datadog monitors.
When default_tags is set at the provider level, resources that do
not otherwise define those tags (by key) will inherit them
automatically during plan.
If the same tag (e.g. "foo:bar", "foo:not_bar") is defined at
provider level and at resource level, the resource tag is used.
@Amaury-Behague
Copy link
Contributor Author

/merge

@dd-devflow
Copy link

dd-devflow bot commented Jul 22, 2024

🚂 MergeQueue: waiting for PR to be ready

This merge request is not mergeable yet, because of pending checks/missing approvals. It will be added to the queue as soon as checks pass and/or get approvals.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.

Use /merge -c to cancel this operation!

@dd-devflow
Copy link

dd-devflow bot commented Jul 22, 2024

⚠️ MergeQueue: This merge request was unqueued

This merge request was unqueued

If you need support, contact us on Slack #devflow!

@Amaury-Behague Amaury-Behague removed the request for review from a team July 24, 2024 12:38
@skarimo skarimo changed the title [datadog_monitor] Add a default_tags attribute in the Datadog provider config. [provider] Add a default_tags attribute in the Datadog provider config. Aug 6, 2024
@skarimo
Copy link
Member

skarimo commented Aug 7, 2024

/merge

@dd-devflow
Copy link

dd-devflow bot commented Aug 7, 2024

🚂 MergeQueue: pull request added to the queue

The median merge time in master is 0s.

Use /merge -c to cancel this operation!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants