-
Notifications
You must be signed in to change notification settings - Fork 412
Add granular namespace filters to GCP V2 Terraform Provider #3299
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
base: master
Are you sure you want to change the base?
Add granular namespace filters to GCP V2 Terraform Provider #3299
Conversation
go.mod
Outdated
|
|
||
| require ( | ||
| github.com/DataDog/datadog-api-client-go/v2 v2.47.1-0.20251014135845-22ba517ffd6e | ||
| github.com/DataDog/datadog-api-client-go/v2 v2.47.1-0.20251022152720-0c92261b1332 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is temporary, will go away when we upgrade to a version containing these changes:
| func (r *integrationGcpStsResource) Update(ctx context.Context, request resource.UpdateRequest, response *resource.UpdateResponse) { | ||
| var state integrationGcpStsModel | ||
|
|
||
| var priorState integrationGcpStsModel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed priorState since it was unused. Granted it was checking the prior state was deserializable, but still think this is the right move.
9274680 to
2c8d662
Compare
117d54f to
ad634ce
Compare
| "filters": types.SetType{ | ||
| ElemType: types.StringType, | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just want to highlight that while this field is actually optional in terms of our API, it's required for terraform and I don't think we have a choice here 😞 .
From my understanding/research, this has to do with terraform provider framework v5 vs v6. In v5, you cannot have a definition for nested types, but you can in v6. Unfortunately, Datadog is stuck on v5 until every team updates to v6 and we stop using the terraform "mux" server.
So, ideally, our schema definition would have:
"metric_namespace_configs": schema.SetNestedAttribute{
Optional: true,
Computed: true,
Description: "Configurations for GCP metric namespaces.",
NestedObject: schema.NestedAttributeObject{
Attributes: map[string]schema.Attribute{
"id": schema.StringAttribute{
Required: true,
Description: "The namespace ID.",
},
"disabled": schema.BoolAttribute{
Optional: true,
Description: "Whether the namespace is disabled.",
},
"filters": schema.SetAttribute{
Optional: true,
Description: "Filters for the namespace.",
ElementType: types.StringType,
},
},
},
},
But this fails our tests with a message like "attributes are not allowed in v5" or something of that sort. Note that our code is actually using v6, but the terraform "mux" server verifies our v6 code is backwards-compatible with v5.
Functionally, what this means is that for customers that currently have metric_namespace_configs defined, once they update their terraform version, they'll have to manually add filters = [] to all of their objects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 aligns with https://developer.hashicorp.com/terraform/plugin/terraform-plugin-protocol#protocol-version-6 and this seems like the best case thinking of other options (ex: deprecate this field and make a new block-based one, make filters a top-level attribute, etc)
Unfortunately, Datadog is stuck on v5 until every team updates to v6 and we stop using the terraform "mux" server
what's the/is there a timeline for this OOC?
ad634ce to
9efce2d
Compare
| func (r *integrationGcpStsResource) Create(ctx context.Context, request resource.CreateRequest, response *resource.CreateResponse) { | ||
| var state integrationGcpStsModel | ||
| response.Diagnostics.Append(request.Plan.Get(ctx, &state)...) | ||
| var plan integrationGcpStsModel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm :D
a8f5477 to
94ca105
Compare
https://datadoghq.atlassian.net/browse/GCP-2929
Related:
https://github.com/DataDog/datadog-api-spec/pull/4510
https://github.com/DataDog/dd-go/pull/203050
This PR adds support for granular namespace filters to the GCP Integrations V2 terraform provider.
Should be straight-forward, but please take note of this comment, as I could potentially see it as a blocker (although IMO I do not think it's fixable): #3299 (comment)
I have marked the PR with label
breaking-changebc of thisAlso please note we should not merge this until we fix some of our broken metric names