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

[CECO-743] Secret backend configuration #1395

Merged
merged 6 commits into from
Oct 3, 2024

Conversation

tbavelier
Copy link
Member

@tbavelier tbavelier commented Sep 9, 2024

What does this PR do?

Implements the Secrets Backend within global to directly configure Secrets management from the DatadogAgent custom resource, providing helpers to handle RBAC similar to the Helm chart

Motivation

Describe your test plan

The scenarios below are covered by the unit tests and replicate them in e2e manner, verifying functionality. Ensure the new version of the CRD is installed with make install and install the built-operator with make IMG=<replace me> deploy (pre-loading your built image in your cluster with kind load docker-image <replace me> --name <KIND CLUSTER NAME>

Testing env variables

[...]
  global:
    [...]
    secretBackend:
      command: "/readsecret_multiple_providers.sh"
      args: "foo bar"
      timeout: 60

Exec into your Agent pod and assert:

  • agent config | grep secret_backend -A 2 matches the parameters defined in the CR
image

Testing global RBAC and secrets resolution

  • Remove the args and timeout. Add enableGlobalPermissions set to true. Re-deploy your CR
[...]
  global:
    [...]
    secretBackend:
      command: "/readsecret_multiple_providers.sh"
      enableGlobalPermissions: true
  • Deploy the RabbitMQ operator and a sample workload :
    • kubectl apply -f "https://github.com/rabbitmq/cluster-operator/releases/download/v2.9.0/cluster-operator.yml"
    • Apply the manifest below
      apiVersion: rabbitmq.com/v1beta1
      kind: RabbitmqCluster
      metadata:
        name: rabbitmqcluster-sample
        namespace: rabbitmq-system
      spec:
        override:
          statefulSet:
            spec:
              template:
                metadata:
                  annotations:
                    ad.datadoghq.com/rabbitmq.check_names: '["rabbitmq"]'
                    ad.datadoghq.com/rabbitmq.init_configs: "[{}]"
                    ad.datadoghq.com/rabbitmq.instances: |
                      [
                        {
                          "rabbitmq_api_url": "http://%%host%%:15672/api/",
                          "username": "ENC[k8s_secret@rabbitmq-system/rabbitmqcluster-sample-default-user/username]",
                          "password": "ENC[k8s_secret@rabbitmq-system/rabbitmqcluster-sample-default-user/password]"
                        }
                      ]

Assert the following :

  • Both the Agent and DCA are able to access the k8s secret with k auth can-i get -n rabbitmq-system secrets/rabbitmqcluster-sample-default-user --as=system:serviceaccount:system:datadog-agent
    image
  • The Agent is resolving the secret successfully with agent secret inside the node Agent
    image

Testing specific RBAC (roles), its priority over enableGlobalPermissions and binding

  • Ensure rabbitmq-system is within WATCH_NAMESPACE variable if not using global watch scope :
    image
  • Add roles and enable CCR. Re-deploy your CR
[...]
  global:
    [...]
    secretBackend:
      command: "/readsecret_multiple_providers.sh"
      enableGlobalPermissions: true
      roles:
      - namespace: rabbitmq-system
        secrets:
        - "rabbitmqcluster-sample-default-user"

Assert the following :

  • The Agent, DCA and CCR are able to access the k8s secret with k auth can-i get -n rabbitmq-system secrets/rabbitmqcluster-sample-default-user --as=system:serviceaccount:system:datadog-agent
  • The Agent, DCA and CCR are NOT able to access the non-specified k8s secret with k auth can-i get -n rabbitmq-system secrets/rabbitmqcluster-sample-erlang-cookie --as=system:serviceaccount:system:datadog-agent
image * Depending on the (other) features enabled, the DCA might have access to secrets cluster-wide, so in that case, verify the Role and RoleBinding with `secret-backend`

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

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This pull request does not contain a valid label. Please add one of the following labels: bug, enhancement, refactoring, documentation, tooling, dependencies

@tbavelier tbavelier added this to the v1.10.0 milestone Sep 9, 2024
@tbavelier tbavelier added component/controller enhancement New feature or request labels Sep 9, 2024
@codecov-commenter
Copy link

codecov-commenter commented Sep 9, 2024

Codecov Report

Attention: Patch coverage is 85.86957% with 13 lines in your changes missing coverage. Please review.

Project coverage is 48.92%. Comparing base (5e5ce7e) to head (b981128).

Files with missing lines Patch % Lines
...nternal/controller/datadogagent/override/global.go 84.12% 8 Missing and 2 partials ⚠️
internal/controller/datadogagent/merger/rbac.go 57.14% 2 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1395      +/-   ##
==========================================
+ Coverage   48.75%   48.92%   +0.17%     
==========================================
  Files         222      222              
  Lines       19342    19432      +90     
==========================================
+ Hits         9431     9508      +77     
- Misses       9425     9435      +10     
- Partials      486      489       +3     
Flag Coverage Δ
unittests 48.92% <85.86%> (+0.17%) ⬆️

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

Files with missing lines Coverage Δ
api/datadoghq/v2alpha1/datadogagent_types.go 100.00% <ø> (ø)
api/datadoghq/v2alpha1/test/builder.go 91.83% <100.00%> (+0.33%) ⬆️
internal/controller/datadogagent/merger/rbac.go 32.23% <57.14%> (+0.26%) ⬆️
...nternal/controller/datadogagent/override/global.go 50.81% <84.12%> (+8.67%) ⬆️

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 5e5ce7e...b981128. Read the comment docs.

@tbavelier tbavelier mentioned this pull request Sep 9, 2024
7 tasks
@tbavelier tbavelier marked this pull request as ready for review September 9, 2024 15:25
@tbavelier tbavelier requested review from a team as code owners September 9, 2024 15:25
Copy link
Contributor

@fanny-jiang fanny-jiang left a comment

Choose a reason for hiding this comment

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

🎉

@tbavelier
Copy link
Member Author

/merge

@dd-devflow
Copy link

dd-devflow bot commented Oct 3, 2024

🚂 MergeQueue: pull request added to the queue

The median merge time in main is 14m.

Use /merge -c to cancel this operation!

@dd-mergequeue dd-mergequeue bot merged commit cfaff9d into main Oct 3, 2024
26 checks passed
@dd-mergequeue dd-mergequeue bot deleted the tbavelier/secret_backend_rebase branch October 3, 2024 09:02
mftoure pushed a commit that referenced this pull request Oct 3, 2024
* global secret backend config

* tests

* fix DefaultAgentResourceSuffix that changed from apicommon to v2alpha1
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