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

Fix parsing group resource to allow groups with periods #1697

Conversation

adel121
Copy link
Contributor

@adel121 adel121 commented Feb 14, 2025

What does this PR do?

Fixes bug in parsing group resource parsing that prevented parsing groups containing periods correctly.

Motivation

Fix the issue.

Additional Notes

Anything else we should know when reviewing?

Describe your test plan

Use custom DCA image: docker.io/adelhajhassan918/dca-test-tracing:latest (built from main for 7.64)

Same testing plan as this

But, here we should test with a resource of which the api group contains periods.

You should add the labels/annotations as tags configuration for ingresses. For example:

apiVersion: datadoghq.com/v2alpha1
kind: DatadogAgent
metadata:
  name: datadog
  namespace: system
spec:
  global:
    credentials:
      apiSecret:
        secretName: datadog-secret
        keyName: api-key
    kubelet:
      tlsVerify: false
    kubernetesResourcesLabelsAsTags:
      ingresses.networking.k8s.io:
        foo: foo
        bar: bar
  override:
    clusterAgent:
      image:
        name: docker.io/adelhajhassan918/dca-test-tracing:latest

    kubernetesResourcesAnnotationsAsTags:
      daemonsets.apps:
        agent.datadoghq.com/agentspechash: hash
      ingresses.networking.k8s.io:
        test.test: test
        test: test2
---
apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  name: example-ingress
  namespace: system
  annotations:
    test.test: hello
    test: hello2
  labels:
    foo: bar
    bar: baz
spec:
  rules:
    - host: example.com
      http:
        paths:
          - path: /
            pathType: Prefix
            backend:
              service:
                name: example-service
                port:
                  number: 80

Then check the that the tags are found in the cluster agent tagger with agent tagger-list:

=== Entity kubernetes_metadata://networking.k8s.io/ingresses/system/example-ingress ===
== Source workloadmeta-kubernetes_metadata =
=Tags: [bar:baz foo:bar test2:hello2 test:hello]
===

Checklist

  • PR has at least one valid label: bug, enhancement, refactoring, documentation, tooling, and/or dependencies
  • PR has a milestone or the qa/skip-qa label

@adel121 adel121 added the enhancement New feature or request label Feb 14, 2025
@adel121 adel121 added this to the v1.13.0 milestone Feb 14, 2025
@adel121 adel121 requested a review from a team as a code owner February 14, 2025 14:34
@codecov-commenter
Copy link

codecov-commenter commented Feb 14, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 49.30%. Comparing base (1c16822) to head (8e19e49).
Report is 2 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1697      +/-   ##
==========================================
- Coverage   49.33%   49.30%   -0.03%     
==========================================
  Files         219      219              
  Lines       21314    21298      -16     
==========================================
- Hits        10515    10501      -14     
+ Misses      10252    10250       -2     
  Partials      547      547              
Flag Coverage Δ
unittests 49.30% <100.00%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...troller/datadogagent/feature/enabledefault/rbac.go 11.82% <100.00%> (-3.89%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1c16822...8e19e49. Read the comment docs.

@adel121 adel121 force-pushed the adelhajhassan/fix_groupresource_parsing_to_allow_groups_with_periods branch from f0f446b to 8e19e49 Compare February 18, 2025 12:18
@celenechang celenechang merged commit 14fc1bb into main Feb 18, 2025
19 checks passed
@celenechang celenechang deleted the adelhajhassan/fix_groupresource_parsing_to_allow_groups_with_periods branch February 18, 2025 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants