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

Fix ObjectMeta CRD validation #242

Merged
merged 4 commits into from
Aug 31, 2020
Merged

Conversation

mikkeloscar
Copy link
Contributor

@mikkeloscar mikkeloscar commented Aug 26, 2020

As mentioned in #241 the CRD validation for the ObjectMeta fields which we used in various places doesn't work because the schema is just object for the whole metadata field so anything below is dropped.

This PR aims to fix the problem in two ways.

  1. Introduce a new type EmbeddedObjectMetaWithAnnotations which is a slimmed down version of metav1.ObjectMeta that only has Annotations as a field. This has the bonus of only exposing the field which is actually used by the stackset controller for various sub resources e.g. setting a custom annotation on the ingress or service resources (Labels are not used, so no need to allow users to set them for instance).
  2. It uses a patched version of https://github.com/kubernetes-sigs/controller-tools based on this PR: ⚠️ Only prevent validation of top-level ObjectMeta field kubernetes-sigs/controller-tools#395. (I have made a fork and are pulling it from there: mikkeloscar/controller-tools@54e4525, I'm open for suggestions on how to better manage this).
  3. Introduce a new type EmbeddedObjectMeta which is a slimmed down version of metav1.ObjectMeta that has Labels and Annotations. This is used for a newly defined PodTemplateSpec type which is similar to v1.PodTemplateSpec but has EmbeddedObjectMeta instead of metav1.ObjectMeta.

Additionally it drops metav1.ObjectMeta from the HorizontalPodAutoscaler field. This is not in use in the code, so it doesn't make sense to support users setting it. We may want to add it in the future as annotations may be needed for interacting with https://github.com/zalando-incubator/kube-metrics-adapter but we have the Autoscaler field supporting this already. I would like to refactor the HorizontalPodAutoscaler field anyway in the future.

@aermakov-zalando
Copy link
Contributor

What about labels? Do we support those?

@coveralls
Copy link

coveralls commented Aug 26, 2020

Pull Request Test Coverage Report for Build 1696

  • 17 of 23 (73.91%) changed or added relevant lines in 1 file are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.07%) to 68.607%

Changes Missing Coverage Covered Lines Changed/Added Lines %
pkg/core/stack_resources.go 17 23 73.91%
Files with Coverage Reduction New Missed Lines %
pkg/core/stack_resources.go 1 90.74%
Totals Coverage Status
Change from base Build 1633: -0.07%
Covered Lines: 1556
Relevant Lines: 2268

💛 - Coveralls

@muaazsaleem
Copy link
Contributor

muaazsaleem commented Aug 26, 2020

Labels are not used, so no need to allow users to set them for instance

@aermakov-zalando ^ from the issue description

@mikkeloscar mikkeloscar force-pushed the fix-object-meta-crd-validation branch from e779a83 to 494cb01 Compare August 26, 2020 15:43
Signed-off-by: Mikkel Oscar Lyderik Larsen <mikkel.larsen@zalando.de>
@mikkeloscar mikkeloscar force-pushed the fix-object-meta-crd-validation branch from 494cb01 to 7cf30e0 Compare August 26, 2020 15:48
@aryszka
Copy link
Contributor

aryszka commented Aug 26, 2020

👍

@mikkeloscar
Copy link
Contributor Author

mikkeloscar commented Aug 27, 2020

Deployments are failing with this in place:

image

Need to adjust the ObjectMeta of the PodTemplate.

Updated the PR description with the fixes that was done.

mikkeloscar and others added 3 commits August 27, 2020 10:19
Signed-off-by: Mikkel Oscar Lyderik Larsen <mikkel.larsen@zalando.de>
Signed-off-by: Mikkel Oscar Lyderik Larsen <mikkel.larsen@zalando.de>
Signed-off-by: Mikkel Oscar Lyderik Larsen <mikkel.larsen@zalando.de>
@mikkeloscar
Copy link
Contributor Author

👍

1 similar comment
@muaazsaleem
Copy link
Contributor

👍

@mikkeloscar mikkeloscar merged commit 32656d2 into master Aug 31, 2020
@mikkeloscar mikkeloscar deleted the fix-object-meta-crd-validation branch August 31, 2020 12:14
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.

5 participants