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(cert-manager): additional options for targeting WebhookConfigurations and CRDs #1276

Merged
merged 26 commits into from
May 4, 2023

Conversation

bacherfl
Copy link
Member

@bacherfl bacherfl commented Apr 25, 2023

This PR adds the additional configuration option of strictly defining which Mutating/Validating Webhooks and CustomResourceDefinitions should be retrieved and extended with the generated certificate.
This can be done by using the following struct when creating a KeptnWebhookCertificateReconciler:

type ObservedObjects struct {
	MutatingWebhooks          []string
	ValidatingWebhooks        []string
	CustomResourceDefinitions []string
	Deployments               []string
}

type CertificateReconcilerConfig struct {
	WatchResources *ObservedObjects // if this is set, the controller will use the exact resources specified here, instead of using the label selector
	MatchLabels    labels.Set
	Namespace      string
	Log            logr.Logger
	Client         client.Client
	Scheme         *runtime.Scheme
	CancelMgrFunc  context.CancelFunc
}

This is intended for use cases where the KeptnWebhookCertificateReconciler will be used as a library in other operators. Using the newly introduced options, operators using this will not require get,list,update permissions for all WebhookConfigurations and CustomResourceDefinitions, but can rely on ClusterRoles giving them access to a strictly defined set of resources, specified by the resourceNames in the ClusterRoles

@netlify
Copy link

netlify bot commented Apr 25, 2023

Deploy Preview for keptn-lifecycle-toolkit ready!

Name Link
🔨 Latest commit 2f63552
🔍 Latest deploy log https://app.netlify.com/sites/keptn-lifecycle-toolkit/deploys/645238fdf3a53700081b8637
😎 Deploy Preview https://deploy-preview-1276--keptn-lifecycle-toolkit.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@codecov
Copy link

codecov bot commented Apr 25, 2023

Codecov Report

Merging #1276 (bd7d39c) into main (737d478) will increase coverage by 0.21%.
The diff coverage is 75.40%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1276      +/-   ##
==========================================
+ Coverage   61.22%   61.44%   +0.21%     
==========================================
  Files         138      143       +5     
  Lines       10471    10733     +262     
==========================================
+ Hits         6411     6595     +184     
- Misses       3807     3878      +71     
- Partials      253      260       +7     
Impacted Files Coverage Δ
...llers/keptnwebhookcontroller/certificate_secret.go 86.04% <50.00%> (ø)
klt-cert-manager/pkg/certificates/watcher.go 59.74% <59.74%> (ø)
klt-cert-manager/eventfilter/eventfilter.go 44.44% <60.00%> (+19.44%) ⬆️
...okcontroller/keptnwebhookcertificate_controller.go 45.39% <65.38%> (+1.33%) ⬆️
klt-cert-manager/pkg/webhook/builder.go 83.78% <83.78%> (ø)
...llers/keptnwebhookcontroller/resource_retriever.go 86.95% <86.95%> (ø)
...ert-manager/pkg/certificates/certificatehandler.go 100.00% <100.00%> (ø)
klt-cert-manager/pkg/webhook/manager.go 100.00% <100.00%> (ø)

... and 6 files with indirect coverage changes

Flag Coverage Δ
certificate-operator 68.55% <75.40%> (+4.26%) ⬆️
component-tests 59.87% <ø> (-0.29%) ⬇️
lifecycle-operator 82.48% <ø> (-0.05%) ⬇️
metrics-operator 77.90% <ø> (+0.09%) ⬆️
scheduler 21.75% <ø> (ø)

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

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Apr 27, 2023
…config

# Conflicts:
#	klt-cert-manager/controllers/keptnwebhookcontroller/webhook_cert_controller_test.go
#	klt-cert-manager/go.mod
#	klt-cert-manager/go.sum
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
# Conflicts:
#	klt-cert-manager/go.mod
Signed-off-by: Florian Bacher <[email protected]>
@bacherfl bacherfl marked this pull request as ready for review May 3, 2023 10:47
@bacherfl bacherfl requested a review from a team as a code owner May 3, 2023 10:47
Signed-off-by: Florian Bacher <[email protected]>
@odubajDT
Copy link
Contributor

odubajDT commented May 3, 2023

I guess you need to rebase to master to make the helm validation check to pass

odubajDT
odubajDT previously approved these changes May 4, 2023
thisthat
thisthat previously approved these changes May 4, 2023
klt-cert-manager/README.md Outdated Show resolved Hide resolved
Co-authored-by: Giovanni Liva <[email protected]>
Signed-off-by: Florian Bacher <[email protected]>
@bacherfl bacherfl dismissed stale reviews from thisthat and odubajDT via bd7d39c May 4, 2023 11:43
@sonarcloud
Copy link

sonarcloud bot commented May 4, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@bacherfl bacherfl merged commit dadd70b into keptn:main May 4, 2023
@keptn-bot keptn-bot mentioned this pull request May 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cert-manager documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants