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

[datadog_monitor] Allow default_tags values to contain colons. #2703

Merged
merged 2 commits into from
Dec 9, 2024

Conversation

Amaury-Behague
Copy link
Contributor

Only separate the key before the first colon, all subsequent colons are
considered to be part of the value as stated in datadog docs.

@Amaury-Behague Amaury-Behague force-pushed the fix-default-tags-colon-values branch from af4f4e5 to e58ffb0 Compare December 4, 2024 12:33
@Amaury-Behague Amaury-Behague marked this pull request as ready for review December 4, 2024 12:46
@Amaury-Behague Amaury-Behague requested review from a team as code owners December 4, 2024 12:46
@Amaury-Behague Amaury-Behague changed the title [datadog-monitor] Allow default_tags values to contain colons (bugfix). [datadog_monitor] Allow default_tags values to contain colons. Dec 4, 2024
@@ -470,12 +470,14 @@ func tagDiff(ctx context.Context, d *schema.ResourceDiff, meta interface{}) erro
kv := strings.Split(tag.(string), ":")
Copy link
Member

Choose a reason for hiding this comment

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

You could probably use strings.Cut (or even strings.SplitN) and avoid having to strings.Join afterwards. This would probably allow to simplify the whole check logic!

Copy link
Member

@LorisFriedel LorisFriedel Dec 4, 2024

Choose a reason for hiding this comment

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

Like so:

    tagStr := tag.(string)
    if tagStr == "" {
        return fmt.Errorf("invalid tag: '%s'", tag)
    }
    key, value, found := strings.Cut(tagStr, ":")
    if !found {
        value = ""
    }
    tags[key] = value

Or even omit the check if !found given that the default value of a string is "" anyway!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point - I always forget about this function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done - I've even removed the special case for empty tags because it does not break the code.

Only separate the key before the first colon, all subsequent colons are
considered to be part of the value as stated in datadog docs.
@Amaury-Behague Amaury-Behague force-pushed the fix-default-tags-colon-values branch from da8db65 to 1a580f5 Compare December 4, 2024 13:42
@nkzou nkzou merged commit 88126a1 into master Dec 9, 2024
12 checks passed
@nkzou nkzou deleted the fix-default-tags-colon-values branch December 9, 2024 15:58
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.

4 participants