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

Support Validating Webhook Configuration resource in K8s package #719

Conversation

Vandit1604
Copy link
Contributor

fixes: #718

We aren't supporting ValidatingWebhookConfiguration but using it in our cluster-infra manifest files for prombench.

@Vandit1604 Vandit1604 force-pushed the feat/support-validating-webhook-configurations branch from 8c798bd to 4cfb338 Compare August 9, 2024 20:51
@Vandit1604 Vandit1604 force-pushed the feat/support-validating-webhook-configurations branch from 4cfb338 to 7bda4bc Compare August 9, 2024 21:02
@Vandit1604
Copy link
Contributor Author

Vandit1604 commented Aug 9, 2024

I guess circleci failed because of my committed go.mod and go.sum. Removed them from the PR to check if that's the case.

@yomek33
Copy link

yomek33 commented Aug 10, 2024

I've encountered an error while applying a ValidatingWebhookConfiguration:

13:07:44 k8s.go:545: resource created - kind: Job, name: ingress-nginx-admission-patch
13:07:54 provider.go:74: Request for 'running job:ingress-nginx-admission-patch' is done!
13:07:54 k8s.go:703: resource updated - kind: IngressClass, name: nginx
Error parsing commandline arguments: error applying 'manifests/cluster-infra/2_ingress-nginx-controller.yaml' err:resource update failed - kind: ValidatingWebhookConfiguration, name: ingress-nginx-admission: validatingwebhookconfigurations.admissionregistration.k8s.io "ingress-nginx-admission" is invalid: metadata.resourceVersion: Invalid value: 0x0: must be specified for an update
usage: infra kind resource apply

It seems related to missing resourceVersion during an update.

var exists bool
for _, l := range list.Items {
if l.Name == req.Name {
exists = true
Copy link

@yomek33 yomek33 Aug 10, 2024

Choose a reason for hiding this comment

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

I think adding the following code can resolve the issue

if req.ResourceVersion == "" {
	req.ResourceVersion = l.ResourceVersion
}

@Vandit1604
Copy link
Contributor Author

Thanks @yomek33 for pointing this out.

@Vandit1604
Copy link
Contributor Author

Updated with changes. Thanks @yomek33

Copy link
Member

@jesusvazquez jesusvazquez left a comment

Choose a reason for hiding this comment

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

LGTM very related to your previous change in #716

@jesusvazquez jesusvazquez merged commit cf77f51 into prometheus:master Aug 14, 2024
4 checks passed
@Vandit1604 Vandit1604 deleted the feat/support-validating-webhook-configurations branch August 14, 2024 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

err:creating request for unimplimented resource type:validatingwebhookconfiguration
3 participants