Skip to content
This repository has been archived by the owner on Apr 7, 2020. It is now read-only.

Add GCP controlplane webhooks #79

Merged

Conversation

stoyanr
Copy link
Contributor

@stoyanr stoyanr commented May 8, 2019

What this PR does / why we need it:
Adds controlplane webhooks for the GCP provider, including tests.

Special notes for your reviewer:

  • The function GetLoadBalancerIngress has been copied from Gardener since its old version can't be used from webhooks (they don't get *rest.Config injected), and the dependency on Gardener can't be updated (due to the controller runtime version). There is a TODO to remove it when the dependency to Gardener is finally updated.
  • There is a lot of duplication with AWS controlplane webhooks. At least some of it could be eliminated by introducing a generic mutator and a new interface to be implemented by providers. This will be proposed in a future PR.

Release note:

Add GCP controlplane webhooks

@stoyanr stoyanr requested a review from a team as a code owner May 8, 2019 15:28
Copy link
Contributor

@adracus adracus left a comment

Choose a reason for hiding this comment

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

Minor nits.

pkg/webhook/controlplane/handler.go Show resolved Hide resolved
@stoyanr stoyanr force-pushed the add-gcp-controlplane-webhooks branch from daeb4b7 to e2245ef Compare May 9, 2019 11:57
@rfranzke rfranzke added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label May 9, 2019
@gardener-robot-ci-1 gardener-robot-ci-1 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels May 9, 2019
Copy link
Contributor

@rfranzke rfranzke left a comment

Choose a reason for hiding this comment

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

/lgtm

@stoyanr stoyanr requested a review from adracus May 9, 2019 14:01
@rfranzke rfranzke merged commit d4ad76c into gardener-attic:master May 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants