K8s1.14 #292

maxsmythe · 2019-11-06T07:49:26Z

PR migrating to Kubebuilder V2. Needs to have the framework PR reviewed first:

Then I will have to remove the following line from go.mod:

	github.com/open-policy-agent/frameworks/constraint => github.com/maxsmythe/frameworks/constraint v0.0.0-20191106045041-e6de551bda0b

Signed-off-by: Max Smythe <smythe@google.com>

test/Dockerfile

Signed-off-by: Max Smythe <smythe@google.com>

ritazh · 2019-11-07T22:06:37Z

.gitmodules

+[submodule "third_party/frameworks"]
+	path = third_party/frameworks
+	url = https://github.com/open-policy-agent/frameworks.git
+	branch = master


what happens if we have updates in master of frameworks but we haven't updated the dependencies in go.mod of gatekeeper? For example, a new spec update in frameworks/constraint/deploy.

The submodule is only here to provide the CRD yamls for constrainttemplate (gomod doesn't let us sync non-go files).

Eventually we may want to cut release branches as it doesn't look like submodules can sync to tags.

there is also this method to include non-source files: https://github.com/go-modules-by-example/index/blob/master/012_modvendor/README.md#vendoring-your-module-download-cache

oh yea that sucks. so I guess we just need to make sure when we update go.mod for frameworks, we also run git submodule update --remote to update these yamls.

@sozercan thanks for the modvendor tip, I've added a vendor target to the makefile that will let us update the yaml that way, while still having a minimal vendor directory.

Makefile

sozercan · 2019-11-07T23:30:44Z

Looks like side effect of moving to deployments, pod name is randomly generated now (like gatekeeper-controller-manager-8446b57df8-cgcjz), we'll have to update documentation to reflect this

ritazh · 2019-11-07T23:58:16Z

deploy/gatekeeper.yaml

  name: gatekeeper-system
 ---
 apiVersion: apiextensions.k8s.io/v1beta1
 kind: CustomResourceDefinition
 metadata:
+  annotations:
+    controller-gen.kubebuilder.io/version: v0.2.2


let's add libs here:

gatekeeper/deploy/gatekeeper.yaml

Lines 419 to 421 in 4df5adc

targets:

items:

properties:

https://github.com/open-policy-agent/frameworks/blob/e6de551bda0b30151ef166c1ac9f988b585600c7/constraint/config/crds/templates_v1beta1_constrainttemplate.yaml#L47-L50

That will happen automatically once I am no longer overriding frameworks with my local copy. Currently it appears the modvendor "use the cache" solution doesn't respect gomod replace directives when pointing to local files.

Signed-off-by: Max Smythe <smythe@google.com>

maxsmythe · 2019-11-08T03:54:15Z

Thanks for the feedback. I looked through the documentation. I could only find one mention of the controller-manager pod, but updated it to reflect the random name.

Signed-off-by: Max Smythe <smythe@google.com>

…er-gen in Docker image Signed-off-by: Max Smythe <smythe@google.com>

Signed-off-by: Max Smythe <smythe@google.com>

ritazh · 2019-11-12T17:00:28Z

pkg/webhook/certs.go

+func (cr *certRotator) writeWebhookConfig(certPem []byte) error {
+	vwh := &unstructured.Unstructured{}
+	vwh.SetGroupVersionKind(schema.GroupVersionKind{Group: "admissionregistration.k8s.io", Version: "v1beta1", Kind: "ValidatingWebhookConfiguration"})
+	vwhKey := types.NamespacedName{Name: "gatekeeper-validating-webhook-configuration"}


What is the reason for changing the name of this webhookconfig to gatekeeper-validating-webhook-configuration

Per our conversation this is for compatibility with controller-gen

ritazh · 2019-11-12T17:18:48Z

pkg/webhook/certs.go

+var (
+	secretKey = types.NamespacedName{
+		Namespace: namespace,
+		Name:      "gatekeeper-webhook-server-cert",


Same here, is there a reason this secret name is changed? It was gatekeeper-webhook-server-secret

Given that we already have breaking changes (webhook name, StatefulSet -> Service, etc.), we may as well make this conform to the new standards as well.

Also, the name of the certs within the secrets have changed, per #111, so this may be a good way to highlight that.

pkg/webhook/certs.go

Signed-off-by: Max Smythe <smythe@google.com>

ritazh

LGTM

…g output Signed-off-by: Max Smythe <smythe@google.com>

Signed-off-by: Max Smythe <smythe@google.com>

…guration Signed-off-by: Max Smythe <smythe@google.com>

Signed-off-by: Max Smythe <smythe@google.com>

maxsmythe · 2019-11-13T05:15:05Z

Waiting to give an opportunity for people to object to the changes introduced with:

ac7a533

Signed-off-by: Max Smythe <smythe@google.com>

pkg/webhook/certs.go

Signed-off-by: Max Smythe <smythe@google.com>

pkg/webhook/certs.go

Signed-off-by: Max Smythe <smythe@google.com>

pkg/webhook/certs.go

Signed-off-by: Max Smythe <smythe@google.com>

ritazh

LGTM

ekitson · 2019-11-14T00:59:24Z

pkg/webhook/certs.go

 		scheme: mgr.GetScheme(),
+		ctx:    context.Background(),


No need to make a change, but just commenting: to really make this code healthy WRT context, you would need to add a context as the first parameter of AddRotator(...) and so on all the way up to the cmd/ file that originates all of this code and call context.Background() there. However that is a much larger change, so...

Ack, ya there is definitely a larger context cleanup to be done across the code base and maybe in core k8s libraries themselves. Calling this out-of-scope for now.

* Migrate libraries, no manifest/Makefile work Signed-off-by: Max Smythe <smythe@google.com> * Migration to Kubebuilder V2 that works on desktop build Signed-off-by: Max Smythe <smythe@google.com> * Use http submodule repo Signed-off-by: Max Smythe <smythe@google.com> * Fix e2e tests Signed-off-by: Max Smythe <smythe@google.com> * fix e2e tests Signed-off-by: Max Smythe <smythe@google.com> * Restore --pull flag in docker-build Signed-off-by: Max Smythe <smythe@google.com> * Use unstructured to inject CA cert to avoid clobbering unknown fields Signed-off-by: Max Smythe <smythe@google.com> * Fix encoding error + nits Signed-off-by: Max Smythe <smythe@google.com> * retrieve crd manifests from vendor cache Signed-off-by: Max Smythe <smythe@google.com> * Fix nits Signed-off-by: Max Smythe <smythe@google.com> * Fix location of CRD yaml in ConstraintTemplate test Signed-off-by: Max Smythe <smythe@google.com> * Use vendor directory exclusively for builds, cache making of controller-gen in Docker image Signed-off-by: Max Smythe <smythe@google.com> * More -mod vendor flags set Signed-off-by: Max Smythe <smythe@google.com> * -mod vendor not necessary for go fmt Signed-off-by: Max Smythe <smythe@google.com> * Switch to distroless Signed-off-by: Max Smythe <smythe@google.com> * remove extra parenthesis Signed-off-by: Max Smythe <smythe@google.com> * More build fixing Signed-off-by: Max Smythe <smythe@google.com> * Gomodules was disabled Signed-off-by: Max Smythe <smythe@google.com> * Need to copy go.mod Signed-off-by: Max Smythe <smythe@google.com> * Forgot copy destination Signed-off-by: Max Smythe <smythe@google.com> * Use committed version of framework; move manifest to gatekeeper_kubebuilder_v2.yaml Signed-off-by: Max Smythe <smythe@google.com> * Missed some files Signed-off-by: Max Smythe <smythe@google.com> * Cert deadlines moved to constants Signed-off-by: Max Smythe <smythe@google.com> * Avoid storing e2e commands in variables. This enables more legible log output Signed-off-by: Max Smythe <smythe@google.com> * Double e2e test timeouts Signed-off-by: Max Smythe <smythe@google.com> * Do not allow kubectl update to clobber cert in validatingwebhookconfiguration Signed-off-by: Max Smythe <smythe@google.com> * update comments Signed-off-by: Max Smythe <smythe@google.com> * Fix grammar on comment Signed-off-by: Max Smythe <smythe@google.com> * Simplify CA cert injection into validating WH into a pure reconcile loop Signed-off-by: Max Smythe <smythe@google.com> * Fix nits Signed-off-by: Max Smythe <smythe@google.com> * Fix nits, unreachable code Signed-off-by: Max Smythe <smythe@google.com> * Fix nits Signed-off-by: Max Smythe <smythe@google.com> * check for key membership in map Signed-off-by: Max Smythe <smythe@google.com> Signed-off-by: Kaleb Wade <kaleb.wade@newcontext.com>

maxsmythe added 2 commits November 5, 2019 23:50

Migrate libraries, no manifest/Makefile work

f4bf694

Signed-off-by: Max Smythe <smythe@google.com>

Migration to Kubebuilder V2 that works on desktop build

2888bd8

Signed-off-by: Max Smythe <smythe@google.com>

maxsmythe force-pushed the k8s1.14 branch from 39b967d to 2888bd8 Compare November 6, 2019 07:50

maxsmythe added 3 commits November 6, 2019 00:00

Use http submodule repo

7b6cc43

Signed-off-by: Max Smythe <smythe@google.com>

Fix e2e tests

ed9adf4

Signed-off-by: Max Smythe <smythe@google.com>

fix e2e tests

3c7d786

Signed-off-by: Max Smythe <smythe@google.com>

maxsmythe requested review from ritazh and ctab November 6, 2019 09:48

Restore --pull flag in docker-build

1e384bf

Signed-off-by: Max Smythe <smythe@google.com>

sozercan reviewed Nov 7, 2019

View reviewed changes

test/Dockerfile Show resolved Hide resolved

maxsmythe added 2 commits November 6, 2019 22:11

Use unstructured to inject CA cert to avoid clobbering unknown fields

b46dce0

Signed-off-by: Max Smythe <smythe@google.com>

Fix encoding error + nits

4df5adc

Signed-off-by: Max Smythe <smythe@google.com>

ritazh reviewed Nov 7, 2019

View reviewed changes

sozercan reviewed Nov 7, 2019

View reviewed changes

Makefile Show resolved Hide resolved

ritazh reviewed Nov 7, 2019

View reviewed changes

maxsmythe added 2 commits November 7, 2019 19:34

retrieve crd manifests from vendor cache

2f89077

Signed-off-by: Max Smythe <smythe@google.com>

Fix nits

22878a6

Signed-off-by: Max Smythe <smythe@google.com>

maxsmythe added 10 commits November 7, 2019 21:30

Fix location of CRD yaml in ConstraintTemplate test

3aa4e59

Signed-off-by: Max Smythe <smythe@google.com>

Use vendor directory exclusively for builds, cache making of controll…

ac0b75c

…er-gen in Docker image Signed-off-by: Max Smythe <smythe@google.com>

More -mod vendor flags set

411189d

Signed-off-by: Max Smythe <smythe@google.com>

-mod vendor not necessary for go fmt

7084a6c

Signed-off-by: Max Smythe <smythe@google.com>

Switch to distroless

65e6c65

Signed-off-by: Max Smythe <smythe@google.com>

remove extra parenthesis

bd7c5cb

Signed-off-by: Max Smythe <smythe@google.com>

More build fixing

3c3f725

Signed-off-by: Max Smythe <smythe@google.com>

Gomodules was disabled

ad1d197

Signed-off-by: Max Smythe <smythe@google.com>

Need to copy go.mod

c3711bf

Signed-off-by: Max Smythe <smythe@google.com>

Forgot copy destination

29af827

Signed-off-by: Max Smythe <smythe@google.com>

maxsmythe requested a review from ritazh November 9, 2019 00:48

ritazh reviewed Nov 12, 2019

View reviewed changes

pkg/webhook/certs.go Outdated Show resolved Hide resolved

Cert deadlines moved to constants

c1e4bba

Signed-off-by: Max Smythe <smythe@google.com>

ritazh approved these changes Nov 12, 2019

View reviewed changes

sozercan approved these changes Nov 12, 2019

View reviewed changes

maxsmythe added 5 commits November 12, 2019 15:08

Avoid storing e2e commands in variables. This enables more legible lo…

be6ad32

…g output Signed-off-by: Max Smythe <smythe@google.com>

Double e2e test timeouts

d7b671c

Signed-off-by: Max Smythe <smythe@google.com>

Do not allow kubectl update to clobber cert in validatingwebhookconfi…

ac7a533

…guration Signed-off-by: Max Smythe <smythe@google.com>

update comments

aa8dd32

Signed-off-by: Max Smythe <smythe@google.com>

Fix grammar on comment

3f6e45d

Signed-off-by: Max Smythe <smythe@google.com>

Simplify CA cert injection into validating WH into a pure reconcile loop

8c67c45

Signed-off-by: Max Smythe <smythe@google.com>

ekitson reviewed Nov 13, 2019

View reviewed changes

pkg/webhook/certs.go Outdated Show resolved Hide resolved

pkg/webhook/certs.go Outdated Show resolved Hide resolved

pkg/webhook/certs.go Outdated Show resolved Hide resolved

pkg/webhook/certs.go Outdated Show resolved Hide resolved

Fix nits

3363394

Signed-off-by: Max Smythe <smythe@google.com>

ritazh reviewed Nov 13, 2019

View reviewed changes

pkg/webhook/certs.go Outdated Show resolved Hide resolved

Fix nits, unreachable code

afaf5a1

Signed-off-by: Max Smythe <smythe@google.com>

ritazh reviewed Nov 14, 2019

View reviewed changes

pkg/webhook/certs.go Outdated Show resolved Hide resolved

ritazh reviewed Nov 14, 2019

View reviewed changes

pkg/webhook/certs.go Outdated Show resolved Hide resolved

ritazh reviewed Nov 14, 2019

View reviewed changes

pkg/webhook/certs.go Show resolved Hide resolved

maxsmythe added 2 commits November 13, 2019 16:16

Fix nits

8c27acb

Signed-off-by: Max Smythe <smythe@google.com>

check for key membership in map

ee135e9

Signed-off-by: Max Smythe <smythe@google.com>

ritazh approved these changes Nov 14, 2019

View reviewed changes

ekitson reviewed Nov 14, 2019

View reviewed changes

maxsmythe merged commit f6e78b3 into open-policy-agent:master Nov 14, 2019

ritazh mentioned this pull request Nov 15, 2019

Deploy gatekeeper validatingwebhookconfiguration to only listen for certain resources? #299

Closed

dmitrytokarev mentioned this pull request Nov 18, 2019

Update to kubebuilder v2 #230

Closed

2 tasks

maxsmythe deleted the k8s1.14 branch December 12, 2019 22:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

K8s1.14 #292

K8s1.14 #292

maxsmythe commented Nov 6, 2019 •

edited

Loading

ritazh Nov 7, 2019

maxsmythe Nov 7, 2019

sozercan Nov 7, 2019

ritazh Nov 7, 2019

maxsmythe Nov 8, 2019

sozercan commented Nov 7, 2019

ritazh Nov 7, 2019

maxsmythe Nov 8, 2019 •

edited

Loading

maxsmythe commented Nov 8, 2019

ritazh Nov 12, 2019

maxsmythe Nov 12, 2019

ritazh Nov 12, 2019

maxsmythe Nov 12, 2019

ritazh left a comment

maxsmythe commented Nov 13, 2019

ritazh left a comment

ekitson Nov 14, 2019

maxsmythe Nov 14, 2019

K8s1.14 #292

K8s1.14 #292

Conversation

maxsmythe commented Nov 6, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sozercan commented Nov 7, 2019

Choose a reason for hiding this comment

maxsmythe Nov 8, 2019 • edited Loading

Choose a reason for hiding this comment

maxsmythe commented Nov 8, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ritazh left a comment

Choose a reason for hiding this comment

maxsmythe commented Nov 13, 2019

ritazh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maxsmythe commented Nov 6, 2019 •

edited

Loading

maxsmythe Nov 8, 2019 •

edited

Loading