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

Cluster Agent Auth Lifecycle Improvment #740

Merged
merged 5 commits into from
Mar 23, 2023

Conversation

CharlyF
Copy link
Contributor

@CharlyF CharlyF commented Mar 23, 2023

What does this PR do?

At the moment, if a token is specified to authenticate the communication between the Datadog Agents and the Datadog Cluster Agent resources will not be rotated if it is changed. This can result in communication errors following up on the upadte of the token.
Secondly, this also adds supports for the conversion of the token when migrating from v1alpha1 to v2alpha1.

This uses the MD5 hashing pattern introduced for Lifecycle improvement.

Motivation

Ensuring smooth migration.

Additional Notes

Anything else we should know when reviewing?

Describe your test plan

Deploying:

apiVersion: datadoghq.com/v2alpha1
kind: DatadogAgent
metadata:
  name: datadog
  namespace: default
spec:
  global:
    clusterAgentToken: abc123 # Make sure it's 32 characters

should result in the creation of the secret with the config hash for the token:

apiVersion: v1
data:
  token: XXXXXXX
kind: Secret
metadata:
  annotations:
    checksum/default-custom-config: 0fe60b5ffa7bdd8c0b26ca3ba0775ff7
  labels:
    app.kubernetes.io/instance: datadog
    app.kubernetes.io/managed-by: datadog-operator
  name: datadog-token
  namespace: default
  ownerReferences:
  - apiVersion: datadoghq.com/v2alpha1
[...]

And both the Datadog Agent and Datadog Cluster Agent pods will have the hash to, ensuring the update if the token is rotated:

➜  ~ k describe pod datadog-cluster-agent-7bf844fcd5-gwtmm
Name:             datadog-cluster-agent-7bf844fcd5-gwtmm
[...]
Labels:           agent.datadoghq.com/component=cluster-agent
[...]
Annotations:      checksum/default-custom-config: 0fe60b5ffa7bdd8c0b26ca3ba0775ff7

(note that this still works if you don't specify the token)

@CharlyF CharlyF requested review from a team as code owners March 23, 2023 04:13
@CharlyF CharlyF added this to the v1.0.0 milestone Mar 23, 2023
@CharlyF CharlyF added bug Something isn't working enhancement New feature or request labels Mar 23, 2023
@codecov-commenter
Copy link

codecov-commenter commented Mar 23, 2023

Codecov Report

Merging #740 (5f6c8c3) into main (1bd631c) will increase coverage by 0.02%.
The diff coverage is 84.21%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #740      +/-   ##
==========================================
+ Coverage   58.83%   58.85%   +0.02%     
==========================================
  Files         149      149              
  Lines       17846    17863      +17     
==========================================
+ Hits        10499    10513      +14     
- Misses       6721     6723       +2     
- Partials      626      627       +1     
Flag Coverage Δ
unittests 58.85% <84.21%> (+0.02%) ⬆️

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

Impacted Files Coverage Δ
controllers/datadogagent/merger/secret.go 60.71% <72.72%> (+7.77%) ⬆️
apis/datadoghq/v1alpha1/datadogagent_conversion.go 81.15% <100.00%> (+0.30%) ⬆️
...rs/datadogagent/feature/externalmetrics/feature.go 41.07% <100.00%> (+0.60%) ⬆️

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 1bd631c...5f6c8c3. Read the comment docs.

Copy link
Collaborator

@clamoriniere clamoriniere left a comment

Choose a reason for hiding this comment

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

small comments to improve a bit the readability of the code.

controllers/datadogagent/feature/enabledefault/feature.go Outdated Show resolved Hide resolved
controllers/datadogagent/feature/enabledefault/feature.go Outdated Show resolved Hide resolved
controllers/datadogagent/merger/secret.go Outdated Show resolved Hide resolved
@CharlyF CharlyF merged commit 5896511 into main Mar 23, 2023
@CharlyF CharlyF deleted the charly/fix-dca-token-conversion branch March 23, 2023 14:56
levan-m pushed a commit that referenced this pull request Mar 23, 2023
* Adding conversion of DCA token

* Adding MD5 hash for DCA token

* Adding hash to components' annotations
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants