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

Convert to using beta resources. #190

Merged
merged 10 commits into from
Jul 25, 2019
Merged

Conversation

maxsmythe
Copy link
Contributor

This also adds in auto-generation of the deploy/gatekeeper.yaml file.

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

This also adds in auto-generation of the deploy/gatekeeper.yaml file.

Signed-off-by: Max Smythe <smythe@google.com>
@maxsmythe maxsmythe requested a review from ritazh July 22, 2019 19:28
Signed-off-by: Max Smythe <smythe@google.com>
@@ -26,7 +26,7 @@ var log = logf.Log.WithName("controller").WithValues("metaKind", "audit")

const (
crdName = "constrainttemplates.templates.gatekeeper.sh"
constraintsGV = "constraints.gatekeeper.sh/v1alpha1"
constraintsGV = "constraints.gatekeeper.sh/v1beta1"
Copy link
Member

Choose a reason for hiding this comment

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

What if users have both v1alpha1 and v1beta1 constraints in the system? This update will only look for the v1beta1 constraints.

Copy link
Contributor Author

@maxsmythe maxsmythe Jul 23, 2019

Choose a reason for hiding this comment

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

Story time :)

This isn't an issue, as all constraints are conceptually simultaneously both alpha and beta at the same time. If a user writes a constraint, it gets stored at the current storage version, when constraints are read, they are read at the requested version.

However...

While verifying this, I found a separate issue. There is a bug in multi-version CRDs such that when:

  1. Admission Webhooks are enabled and registered (GK is an admission webhook)
  2. A cluster has a CR persisted in an old storage version
  3. A cluster upgrades the CRD and bumps the storage version

It is impossible to write a beta version to a resource persisted in alpha. The API server tries to do a (not-unstructured) scheme conversion and errors out. Fortunately it's possible to bump the storage version by writing an alpha version. I've added some code to do that on startup in pkg/upgrade.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Posted bug to k8s repo:

kubernetes/kubernetes#80494

…raints/templates

Signed-off-by: Max Smythe <smythe@google.com>
Signed-off-by: Max Smythe <smythe@google.com>
Signed-off-by: Max Smythe <smythe@google.com>
Signed-off-by: Max Smythe <smythe@google.com>
@maxsmythe maxsmythe requested a review from ritazh July 23, 2019 22:56
Signed-off-by: Max Smythe <smythe@google.com>
.gitignore Outdated Show resolved Hide resolved
Signed-off-by: Max Smythe <smythe@google.com>
subjects:
- kind: ServiceAccount
name: default
namespace: system
Copy link
Member

@ritazh ritazh Jul 24, 2019

Choose a reason for hiding this comment

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

make deploy/kustomize updates the namespace value to gatekeeper-system. Is there a reason this was updated to system?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch.

For me, make deploy is leaving the namespace as system. I think this may have been a change in Kustomize behavior. I've amended the config so it writes gatekeeper-system.

Can you verify that when you run make manifests with the latest version of this change gatekeeper.yaml is unchanged for you?

Copy link
Member

@ritazh ritazh Jul 25, 2019

Choose a reason for hiding this comment

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

Running make manifests updates rbac_role_binding.yaml to namespace: system and deploy/gatekeeper.yaml to namespace: gatekeeper-system

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so the namespace correctly points to gatekeeper-system for you in gatekeeper.yaml?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is what I get for running kustomize from HEAD.

Should be fixed now.

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

@ritazh ritazh left a comment

Choose a reason for hiding this comment

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

LGTM

@maxsmythe
Copy link
Contributor Author

Woohoo! Pushing.

@maxsmythe maxsmythe merged commit fa8035b into open-policy-agent:master Jul 25, 2019
@maxsmythe maxsmythe deleted the beta branch December 12, 2019 22:24
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.

None yet

2 participants