-
Notifications
You must be signed in to change notification settings - Fork 53
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
Add v1beta1 versions of Constraints and ConstraintTemplates #39
Conversation
constraint/pkg/apis/templates/v1beta1/constrainttemplate_types_test.go
Outdated
Show resolved
Hide resolved
h.scheme.Default(crd) | ||
crd.ObjectMeta.Name = fmt.Sprintf("%s.%s", crd.Spec.Names.Plural, constraintGroup) | ||
return crd | ||
// Defaulting functions only exist for v1beta1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you provide more information for this? why do we need to convert crd to apiextensionsv1beta1
then apiextensions
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The kubernetes scheme is documented here:
https://godoc.org/k8s.io/apimachinery/pkg/runtime#Scheme
I've tried to use the unversioned, internal representation of the CRD types in an effort to make the API a bit more stable.
The CRD library has chosen to only register defaulting functions (those that set defaults on a type) for the versioned verision of the CRD resource. This function takes an unversioned schema, which corresponds to the unversioned CRD.
This means I have two options:
- Convert the schema to a versioned JSONSchema, implement a versioned CRD, apply defaults, convert to unversioned.
- Create an unversioned CRD, convert to versioned CRD, apply defaults, convert to unversioned CRD
I expect choice (2) will require less change if/when CRD v2 comes around.
@@ -118,8 +142,8 @@ func (h *crdHelper) validateCR(cr *unstructured.Unstructured, crd *apiextensions | |||
if cr.GroupVersionKind().Group != constraintGroup { | |||
return fmt.Errorf("Wrong group for constraint %s. Have %s, want %s", cr.GetName(), cr.GroupVersionKind().Group, constraintGroup) | |||
} | |||
if cr.GroupVersionKind().Version != crd.Spec.Version { | |||
return fmt.Errorf("Wrong version for constraint %s. Have %s, want %s", cr.GetName(), cr.GroupVersionKind().Version, crd.Spec.Version) | |||
if !supportedVersions[cr.GroupVersionKind().Version] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this check against the supported versions of constraints.gatekeeper.sh
not templates.gatekeeper.sh
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm assuming these will be 1:1, but that's not necessarily guaranteed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is probably ok for now, but we might need to come back to this later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ack, LMK if you'd much rather prefer I add some constants somewhere. I don't really have a preference personally.
Left few comments, but otherwise LGTM. |
Note that this also required moving to a model where we use a common core representation of the template resource and implement a method for conversion between each version. Another issue was that the version of controller-gen that ships with Kubebuilder v1 doesn't handle multi-version CRDs, which I've addressed by patching the versions in via kustomize. Signed-off-by: Max Smythe <[email protected]>
Signed-off-by: Max Smythe <[email protected]>
Signed-off-by: Max Smythe <[email protected]>
Signed-off-by: Max Smythe <[email protected]>
Signed-off-by: Max Smythe <[email protected]>
Signed-off-by: Chris Aniszczyk <[email protected]>
Note that this also required moving to a model where we use a common
core representation of the template resource and implement a method for
conversion between each version.
Another issue was that the version of controller-gen that ships
with Kubebuilder v1 doesn't handle multi-version CRDs, which I've
addressed by patching the versions in via kustomize.
Signed-off-by: Max Smythe [email protected]