CON-7851\Register Lb validation handler #586

elijahomolo · 2023-02-09T23:12:18Z

https://jira.internal.digitalocean.com/browse/CON-7851

This PR addresses CON-7851 by adding a new validation handler to handle LB type requests. This PR only adds the functionality to parse the LB service type and pass Create and Update requests to the appropriate LBAAS DO API endpoints. A followup PR will add validation for individual configurations.

elijahomolo · 2023-02-10T00:16:37Z

PR to fix CI: #585

timoreimann

I'd appreciate if we could introduce a flag / environment variable at this point already that causes the webhook to not do anything (and log clearly about it) unless it is enabled specifically.

This is to guard against users eventually discovered our (unfinished) work and trying it out, possibly creating more tickets due to confusion.

cloud-controller-manager/cmd/digitalocean-cloud-controller-manager/main.go

cloud-controller-manager/cmd/digitalocean-webhook-server/Dockerfile

cloud-controller-manager/cmd/digitalocean-webhook-server/main.go

cloud-controller-manager/do/doks_lb_validator.go

cloud-controller-manager/do/doks_lb_validator_test.go

tmp/webhook_manifests/manifest.yaml

tmp/webhook_manifests/webhook-rolebinding.yaml

cloud-controller-manager/do/doks_lb_validator.go

MorrisLaw · 2023-03-21T14:10:55Z

go.mod

@@ -4,7 +4,8 @@ go 1.19

 require (
 github.com/davecgh/go-spew v1.1.1
- github.com/digitalocean/godo v1.97.0
+ github.com/digitalocean/godo v1.97.1-0.20230308191258-ca2138d84446


This should be 1.98. Change might've got lost in the rebase here. Pretty sure he had it at one point.

cloud-controller-manager/do/kubernetes_lb_validator.go

cloud-controller-manager/cmd/digitalocean-webhook-server/main.go

cloud-controller-manager/do/kubernetes_lb_validator.go

timoreimann · 2023-03-29T15:11:33Z

cloud-controller-manager/do/doks_lb_validator.go

+ }
+
+ err = v.decoder.DecodeRaw(req.OldObject, svc)
+ if err != nil {


The IP would only be available once an LB is fully provisioned, so I'm not sure we can use that.

Can you check if status.loadBalancer.ingress exists right after a managed LB is created? If it does even though the IP address is still missing, then we could use that as a trigger.

The other possibility is to check the LB ID annotation, though using something on the status should be a bit more reliable.

timoreimann · 2023-03-29T15:12:34Z

cloud-controller-manager/do/doks_lb_validator.go

+ }
+
+ err = v.decoder.DecodeRaw(req.OldObject, svc)
+ if err != nil {


Can you answer this question of mine:

Is req.OldObject possibly nil when the Service is brand new?

cloud-controller-manager/do/kubernetes_lb_validator.go

cloud-controller-manager/cmd/digitalocean-webhook-server/main.go

cloud-controller-manager/do/kubernetes_lb_validator.go

cloud-controller-manager/do/kubernetes_lb_validator_test.go

cloud-controller-manager/do/kubernetes_lb_validator.go

timoreimann

We should also have a very clear note/disclaimer about the state of the web hook, i.e., that it is purely experimental yet. I suggest we

add a README.md right next to the main.go file
add a loud and clear log message at the start of the webhook
add a comment header in the main.go file

cloud-controller-manager/cmd/digitalocean-webhook-server/Dockerfile

cloud-controller-manager/cmd/digitalocean-webhook-server/main.go

cloud-controller-manager/do/kubernetes_lb_validator.go

cloud-controller-manager/cmd/digitalocean-webhook-server/Dockerfile

cloud-controller-manager/cmd/digitalocean-webhook-server/README.md

cloud-controller-manager/cmd/digitalocean-webhook-server/main.go

cloud-controller-manager/cmd/digitalocean-webhook-server/Dockerfile

cloud-controller-manager/cmd/digitalocean-webhook-server/main.go

cloud-controller-manager/do/kubernetes_lb_validator.go

cloud-controller-manager/cmd/digitalocean-webhook-server/main.go

cloud-controller-manager/do/kubernetes_lb_validator.go

timoreimann · 2023-05-24T09:03:35Z

@elijahomolo you seem to have merge commit markers in your go.mod file now. You need to remove them and run go mod tidy && go mod vendor afterwards to be safe.

timoreimann · 2024-05-27T22:24:09Z

Closing in favor of #676 and several follow-up PRs that were submitted/merged by now to continue and complete the validation web hook work.

elijahomolo force-pushed the lb-validation-handler branch from f8c98a4 to f32aad7 Compare February 22, 2023 16:50

elijahomolo force-pushed the lb-validation-handler branch from c94c22b to 522db10 Compare March 9, 2023 21:12

elijahomolo force-pushed the lb-validation-handler branch from 4b0965e to 5fe2f02 Compare March 20, 2023 15:57

elijahomolo marked this pull request as ready for review March 20, 2023 16:02

elijahomolo requested review from MorrisLaw, varshavaradarajan and timoreimann March 20, 2023 16:03

timoreimann reviewed Mar 21, 2023

View reviewed changes

timoreimann force-pushed the lb-validation-handler branch from 5fe2f02 to 7571254 Compare March 21, 2023 08:47

elijahomolo force-pushed the lb-validation-handler branch from 7571254 to f6abbfb Compare March 21, 2023 13:26

MorrisLaw reviewed Mar 21, 2023

View reviewed changes

elijahomolo force-pushed the lb-validation-handler branch 2 times, most recently from e2a673b to 17d2f0c Compare March 27, 2023 15:33

MorrisLaw reviewed Mar 27, 2023

View reviewed changes

cloud-controller-manager/do/kubernetes_lb_validator.go Outdated Show resolved Hide resolved

elijahomolo force-pushed the lb-validation-handler branch from 82e0b0f to 1718245 Compare March 27, 2023 20:58

elijahomolo requested review from timoreimann and MorrisLaw March 28, 2023 17:32

timoreimann reviewed Mar 29, 2023

View reviewed changes

elijahomolo force-pushed the lb-validation-handler branch from 4ce49ca to af13250 Compare April 6, 2023 15:25

varshavaradarajan requested changes Apr 12, 2023

View reviewed changes

varshavaradarajan requested changes Apr 17, 2023

View reviewed changes

cloud-controller-manager/do/kubernetes_lb_validator.go Outdated Show resolved Hide resolved

timoreimann reviewed Apr 18, 2023

View reviewed changes

timoreimann reviewed Apr 20, 2023

View reviewed changes

cloud-controller-manager/do/kubernetes_lb_validator.go Outdated Show resolved Hide resolved

cloud-controller-manager/do/kubernetes_lb_validator.go Outdated Show resolved Hide resolved

cloud-controller-manager/do/kubernetes_lb_validator.go Outdated Show resolved Hide resolved

timoreimann reviewed Apr 20, 2023

View reviewed changes

cloud-controller-manager/do/kubernetes_lb_validator.go Outdated Show resolved Hide resolved

timoreimann reviewed Apr 24, 2023

View reviewed changes

timoreimann reviewed Apr 25, 2023

View reviewed changes

elijahomolo requested a review from timoreimann April 28, 2023 13:47

elijahomolo force-pushed the lb-validation-handler branch from 4f5040d to 7e55ff6 Compare April 28, 2023 13:50

timoreimann reviewed Apr 28, 2023

View reviewed changes

elijahomolo added 22 commits May 23, 2023 12:06

remove unused function

e7418c5

assert on admission response reason or message

bf18461

remove obsolete log options

d476ace

remove returned bool from isValidationError and rename to getStatusCode

36f2a3c

refactor isValidationError to function

2a5046e

remove unneccesary formatting

ca1dc80

refactor buildRequest to function

7288f5b

separate svc type and deleting conditions

74548e5

ammend failed validation message

0bfeb81

ammend invalid update message and admission response

4bab1dc

rename new svc in test cases

d547604

test for condition where LB ip exists but no annotation

5af5b25

upgade assertion format

42451b1

change print formatting

74d83ae

add context to fake error response body

de0f671

assert on status code where admission response is an error

4bb07e4

simplify test assertions

7ce42a8

formatting

b54c02f

set region as constant

b1e8b75

log info NIT

74e4750

update fake svc name in tests

9c8385e

move region set to webook server init

d083584

elijahomolo force-pushed the lb-validation-handler branch from 839ff25 to d083584 Compare May 23, 2023 17:37

elijahomolo added 4 commits May 24, 2023 09:53

fix vendoring

5b15c7a

only request from metadata during initialiation

e3395b1

formatting NIT

0ae4f09

add vendor packages

fdd3f1c

timoreimann closed this May 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CON-7851\Register Lb validation handler #586

CON-7851\Register Lb validation handler #586

elijahomolo commented Feb 9, 2023 •

edited

elijahomolo commented Feb 10, 2023

timoreimann left a comment

MorrisLaw Mar 21, 2023

timoreimann Mar 29, 2023

timoreimann Mar 29, 2023

timoreimann left a comment

timoreimann commented May 24, 2023

timoreimann commented May 27, 2024

CON-7851\Register Lb validation handler #586

CON-7851\Register Lb validation handler #586

Conversation

elijahomolo commented Feb 9, 2023 • edited

elijahomolo commented Feb 10, 2023

timoreimann left a comment

Choose a reason for hiding this comment

MorrisLaw Mar 21, 2023

Choose a reason for hiding this comment

timoreimann Mar 29, 2023

Choose a reason for hiding this comment

timoreimann Mar 29, 2023

Choose a reason for hiding this comment

timoreimann left a comment

Choose a reason for hiding this comment

timoreimann commented May 24, 2023

timoreimann commented May 27, 2024

elijahomolo commented Feb 9, 2023 •

edited