-
Notifications
You must be signed in to change notification settings - Fork 116
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
yaml/v2: apply the namespace uniformly to the resource name #2897
Conversation
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.
LGTM with 1 minor non-blocking feedback.
def34e8
to
ea8093e
Compare
Does the PR have any schema changes?Looking good! No breaking changes found. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2897 +/- ##
==========================================
+ Coverage 27.19% 27.55% +0.35%
==========================================
Files 53 53
Lines 7780 7818 +38
==========================================
+ Hits 2116 2154 +38
+ Misses 5486 5485 -1
- Partials 178 179 +1 ☔ View full report in Codecov by Sentry. |
Unfortunately there's a problem with this, in preview mode, the CRDs aren't actually installed and this breaks multi-CG use cases. The solution will either be to eat the error in preview mode, or to somehow resolve the CRDs across components. A follow-up PR: |
Proposed changes
This PR strengthens the normalization of the parsed Kubernetes objects to better interoperate with #2881.
Specifically:
depends-on
resource.default/my-map
versusmy-map
).Register
.For example, given the below manifest and assuming that the default namespace is
default
, we would expect aDependsOn
option fromCertificate
toIssuer
. Observe thatnamespace
isn't explicitly set on the issuer.To apply the default namespace correctly, one must know whether a given kind is namespace-scoped. The provider naturally uses the discovery client, and also looks for a matching CRD amongst the parsed objects (since they're not yet installed).
Testing
New test cases were added for
Normalize
, and some test cases were moved fromRegister
because the validation is performed more eagerly.New sub-tests were added for
IsNamespacedKind
to cover the local CRD enhancement.Related issues (optional)
Closes #2870