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

feat(admission controller): Add new webhook settings #1461

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

wdhif
Copy link
Member

@wdhif wdhif commented Oct 11, 2024

What does this PR do?

Add support for the new Cluster Agent Admission settings.

Motivation

Needed to support new Cluster Agent settings.

Minimum Agent Versions

Are there minimum versions of the Datadog Agent and/or Cluster Agent required?

  • Cluster Agent: v7.59.0

Describe your test plan

Using the following settings:

  features:
    admissionController:
      enabled: true
      validation:
        enabled: false
      mutation:
        enabled: false

Make sure that the output of those commands is as follows:

➜ kubectl get mutatingwebhookconfigurations.admissionregistration.k8s.io -A
No resources found
➜ kubectl get validatingwebhookconfigurations.admissionregistration.k8s.io -A
No resources found

Modify the setting with:

  features:
    admissionController:
      enabled: true
      validation:
        enabled: true
      mutation:
        enabled: true

Make sure that the output changes to:

➜ kubectl get validatingwebhookconfigurations.admissionregistration.k8s.io -A
NAME              WEBHOOKS   AGE
datadog-webhook   0          45s
➜ kubectl get mutatingwebhookconfigurations.admissionregistration.k8s.io -A
NAME              WEBHOOKS   AGE
datadog-webhook   3          47s

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

@wdhif wdhif requested a review from a team as a code owner October 11, 2024 08:23
@wdhif wdhif marked this pull request as draft October 11, 2024 08:23
github-actions[bot]

This comment was marked as resolved.

@wdhif wdhif added the enhancement New feature or request label Oct 11, 2024
@wdhif wdhif added this to the v1.10.0 milestone Oct 11, 2024
@wdhif wdhif force-pushed the CONTP-379/wassim.dhif/admission_webhook_settings branch 24 times, most recently from 7f67f67 to 9033d58 Compare October 16, 2024 10:13
@wdhif wdhif force-pushed the CONTP-379/wassim.dhif/admission_webhook_settings branch 12 times, most recently from b40425c to 48bfe1f Compare October 16, 2024 13:55
@codecov-commenter
Copy link

codecov-commenter commented Oct 16, 2024

Codecov Report

Attention: Patch coverage is 95.23810% with 2 lines in your changes missing coverage. Please review.

Project coverage is 48.80%. Comparing base (86c5b1d) to head (951e1f2).

Files with missing lines Patch % Lines
api/datadoghq/v2alpha1/datadogagent_default.go 80.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1461      +/-   ##
==========================================
+ Coverage   48.70%   48.80%   +0.10%     
==========================================
  Files         224      224              
  Lines       19852    19886      +34     
==========================================
+ Hits         9669     9706      +37     
+ Misses       9675     9673       -2     
+ Partials      508      507       -1     
Flag Coverage Δ
unittests 48.80% <95.23%> (+0.10%) ⬆️

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 92.20% <100.00%> (+0.19%) ⬆️
...atadogagent/feature/admissioncontroller/feature.go 80.88% <100.00%> (+1.35%) ⬆️
internal/controller/datadogagent/feature/types.go 26.92% <ø> (ø)
api/datadoghq/v2alpha1/datadogagent_default.go 90.65% <80.00%> (+0.90%) ⬆️

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 86c5b1d...951e1f2. Read the comment docs.

@wdhif wdhif force-pushed the CONTP-379/wassim.dhif/admission_webhook_settings branch 6 times, most recently from 9cb8b16 to 0c2c5d4 Compare October 17, 2024 08:59
@wdhif wdhif marked this pull request as ready for review October 17, 2024 12:29
@wdhif wdhif requested review from a team as code owners October 17, 2024 12:29
Signed-off-by: Wassim DHIF <wassim.dhif@datadoghq.com>
@wdhif wdhif force-pushed the CONTP-379/wassim.dhif/admission_webhook_settings branch from 0c2c5d4 to b97385e Compare October 17, 2024 12:38
Copy link

Choose a reason for hiding this comment

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

Curious. What are some potential future mutating and validation configurations (besides just enabled true/false) that could be used by the user?

Copy link
Member Author

Choose a reason for hiding this comment

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

In the future I would like to relocate the mutate_unlabelled setting to the mutation section (while keeping backward compatibility) and also add a validate_unlabelled one for the validation.

@@ -254,7 +273,7 @@ func testDCAResources(acm string, registry string, cwsInstrumentationEnabled boo
)
}

func getACEnvVars(acm, registry string, cws bool) []*corev1.EnvVar {
func getACEnvVars(validation, mutation bool, acm, registry string, cws bool) []*corev1.EnvVar {
Copy link

Choose a reason for hiding this comment

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

nit: why not add the validation and mutation bool at the end with the cws bool.

Copy link
Member Author

Choose a reason for hiding this comment

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

I considered them to be more "important" as the other ones since they control the main Validation and Mutation functionality.

@fanny-jiang fanny-jiang modified the milestones: v1.10.0, v1.11.0 Oct 28, 2024
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.

6 participants