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] Secrets backend feature #1333

Closed
wants to merge 20 commits into from

Conversation

tbavelier
Copy link
Member

@tbavelier tbavelier commented Jul 29, 2024

What does this PR do?

Implements the Secrets Backend feature 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=tbdatadog/operator:1.9.0 deploy (pre-loading your built image in your cluster with kind load docker-image tbdatadog/operator:1.9.0 --name <KIND CLUSTER NAME>

Testing env variables

[...]
  features:
    [...]
    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
[...]
  features:
    [...]
    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
[...]
  features:
    [...]
     clusterChecks:
      useClusterChecksRunners: true
    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

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

@tbavelier tbavelier added enhancement New feature or request wip Work in progress component/controller do-not-merge labels Jul 29, 2024
@tbavelier tbavelier added this to the v1.9.0 milestone Aug 2, 2024
@codecov-commenter
Copy link

codecov-commenter commented Aug 5, 2024

Codecov Report

Attention: Patch coverage is 82.93839% with 36 lines in your changes missing coverage. Please review.

Project coverage is 47.55%. Comparing base (aebcfac) to head (d05db57).

Files with missing lines Patch % Lines
...lers/datadogagent/feature/secretbackend/feature.go 79.24% 25 Missing and 8 partials ⚠️
controllers/datadogagent/merger/rbac.go 57.14% 2 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1333      +/-   ##
==========================================
+ Coverage   47.14%   47.55%   +0.40%     
==========================================
  Files         217      219       +2     
  Lines       18768    18977     +209     
==========================================
+ Hits         8849     9025     +176     
- Misses       9457     9482      +25     
- Partials      462      470       +8     
Flag Coverage Δ
unittests 47.55% <82.93%> (+0.40%) ⬆️

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

Files with missing lines Coverage Δ
apis/datadoghq/v2alpha1/datadogagent_default.go 89.44% <100.00%> (+0.12%) ⬆️
apis/datadoghq/v2alpha1/datadogagent_types.go 100.00% <ø> (ø)
apis/datadoghq/v2alpha1/test/builder.go 91.87% <100.00%> (+0.43%) ⬆️
controllers/datadogagent/controller.go 53.33% <ø> (ø)
...rollers/datadogagent/feature/secretbackend/rbac.go 100.00% <100.00%> (ø)
controllers/datadogagent/merger/rbac.go 32.23% <57.14%> (+0.26%) ⬆️
...lers/datadogagent/feature/secretbackend/feature.go 79.24% <79.24%> (ø)

... and 1 file with indirect coverage changes


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 aebcfac...d05db57. Read the comment docs.

@tbavelier tbavelier changed the title [Draft] Secrets backend feature [CECO-743] Secrets backend feature Aug 27, 2024
@tbavelier tbavelier removed wip Work in progress do-not-merge labels Aug 27, 2024
@tbavelier tbavelier marked this pull request as ready for review August 27, 2024 10:04
@tbavelier tbavelier requested review from a team as code owners August 27, 2024 10:04
docs/configuration.v2alpha1.md Outdated Show resolved Hide resolved
docs/configuration.v2alpha1.md Outdated Show resolved Hide resolved
docs/configuration.v2alpha1.md Outdated Show resolved Hide resolved
docs/configuration.v2alpha1.md Outdated Show resolved Hide resolved
docs/configuration.v2alpha1.md Outdated Show resolved Hide resolved
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.

Looks great! I only have one comment about the config naming

apis/datadoghq/v2alpha1/datadogagent_types.go Outdated Show resolved Hide resolved
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.

Also, can we move secretBackend from features to global?

@tbavelier
Copy link
Member Author

Closing in favor of #1395 : moving from a feature to spec.global per comment

@tbavelier tbavelier closed this Sep 9, 2024
@khewonc khewonc removed this from the v1.10.0 milestone Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants