-
Notifications
You must be signed in to change notification settings - Fork 112
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
[CONTP-376] expose kubernetes_resources_labels/annotations_as_tags #1379
[CONTP-376] expose kubernetes_resources_labels/annotations_as_tags #1379
Conversation
ok = false | ||
} | ||
|
||
return |
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.
🔵 Code Quality Violation
avoid bare return (...read more)
The "Avoid bare returns" rule in Go static analysis is designed to increase clarity and readability in your code. A bare return is when a function that has named return values returns those values implicitly, without explicitly stating what is being returned.
While Go's allowance for bare returns can make code more concise, it can also make it more difficult to understand and debug, especially in larger functions. Implicitly relying on the state of named return values can lead to unexpected behavior if those values are modified elsewhere in the function.
To adhere to this rule and promote better coding practices, always explicitly return values in your functions. This makes it clear what values are being returned and in what state, reducing the chance of bugs and making your code easier to understand. For example, instead of writing return
in a function that returns an int
and a bool
, write return 0, false
.
Learn More
The new configuration options
The intuition is that the new config is more generic, and it can be used to achieve the exact same functionality as the previous configs, but also extends to other types (e.g. daemonsets, statefulsets, etc.). We can't drop support for the old config options, we should keep them in order not to break backward compatibility, but at the same time we should push users to use the new config. What is the proper way to do this? Should we include comments in the code in the operator? or should we add a hint in the documentation? |
…gs_and_annotations_as_tags
66862d3
to
d8c717b
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1379 +/- ##
==========================================
+ Coverage 49.19% 49.22% +0.02%
==========================================
Files 223 224 +1
Lines 19612 19700 +88
==========================================
+ Hits 9648 9697 +49
- Misses 9461 9497 +36
- Partials 503 506 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
|
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.
good for docs
…assan/expose_kubernetes_labels_as_tags_and_annotations_as_tags
We can update the type hints for those configs and add a note that encourages users to use the new config option. These notes will get autogenerated in the documentation. For example, the docs update would go in the datadog-operator/api/datadoghq/v2alpha1/datadogagent_types.go Lines 1200 to 1201 in fe0188f
|
@@ -82,6 +82,11 @@ func GetExternalMetricsReaderClusterRoleName(dda metav1.Object, versionInfo *ver | |||
return fmt.Sprintf("%s-metrics-reader", GetClusterAgentRbacResourcesName(dda)) | |||
} | |||
|
|||
// GetResourceMetadataAsTagsClusterRoleName returns the name for the cluster role name used for kubernetes resource labels and annotations as tags | |||
func GetResourceMetadataAsTagsClusterRoleName(dda metav1.Object) string { | |||
return fmt.Sprintf("%s-kubernetes-metadata-as-tags", GetClusterAgentRbacResourcesName(dda)) |
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.
I think we should use -annotations-and-labels-as-tags
to stay consistent with what we're doing in the helm chart:
return fmt.Sprintf("%s-kubernetes-metadata-as-tags", GetClusterAgentRbacResourcesName(dda)) | |
return fmt.Sprintf("%s-annotations-and-labels-as-tags", GetClusterAgentRbacResourcesName(dda)) |
…gs_and_annotations_as_tags
What does this PR do?
This PR exposes
kubernetesResourcesLabelsAsTags
andkubernetesResourcesAnnotationsAsTags
in the DD operator.Feature implemented in this PR.
Motivation
Exposing the feature.
Additional Notes
We also create DCA rbacs necessary for the feature to work as expected.
Minimum Agent Versions
Are there minimum versions of the Datadog Agent and/or Cluster Agent required?
Agent: v7.58.0
Cluster Agent: v7.58.0
Describe your test plan
Deploy the following DatadogAgent with the operator:
Create a deployment with these labels and annotations set on the deployment object and the children pods:
Verify that the proper rbacs were created, in this case we should have
get
,list
andwatch
permissions on the following resources:You can verify this by checking the clusterroles that were created, one of them should be related to metadata as tags:
Also verify that the feature is working by checking the content of the tagger.
You should be able to see
bar1:label4 bar:label2 baz:label1 baz:label3
tags on the created deployments and its children pods. And you should see the hash label (e..g.hash:27a53207eedd862383f608e9a6aa1341
) on the datadog-agent daemonset within akubernetes_metadata
entity.Checklist
bug
,enhancement
,refactoring
,documentation
,tooling
, and/ordependencies
qa/skip-qa
label