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

Whitelist recursively transformable kinds. #3833

Merged
merged 1 commit into from
Apr 15, 2020

Conversation

mgoltzsche
Copy link
Contributor

@mgoltzsche mgoltzsche commented Mar 17, 2020

Description
Refers to/fixes #1737, particularly to this comment.

Previously (PR) FieldVisitors (image/namespace collector/transformer, labelsSetter) are applied to all fields of all resources recursively except for CRDs which were known to break otherwise since their schema contains a "metadata" property as well.
However other unknown kinds / CRs may contain such properties as well and break.

This PR whitelists the known kinds that can be transformed recursively instead of attempting to blacklist kinds that cannot be.

User facing changes
Applying a CR that contains a "metadata" or "image" field within the spec will work. (Previously skaffold destroyed the CR.)
Resource label and image modifications are only applied recursively on the fields of a known workload kind: Pod, ReplicaSet, StatefulSet, Deployment, DaemonSet, Job, CronJob.

Refers to issue GoogleContainerTools#1737.

Previously FieldVisitors (image/namespace collector/transformer, labelsSetter) are applied to all fields of all resources recursively except for CRDs which were known to break otherwise since their schema contains a "metadata" property as well.
However other unknown kinds / CRs may contain such properties as well and break.

This PR whitelists the known kinds that can be transformed recursively instead of attempting to blacklist kinds that cannot.

Signed-off-by: Max Goltzsche <[email protected]>
@codecov
Copy link

codecov bot commented Mar 17, 2020

Codecov Report

Merging #3833 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

Impacted Files Coverage Δ
pkg/skaffold/deploy/kubectl/visitor.go 93.54% <100%> (ø) ⬆️
...affold/kubernetes/portforward/kubectl_forwarder.go 65.85% <0%> (-2.44%) ⬇️

@dgageot dgageot added the kokoro:run runs the kokoro jobs on a PR label Mar 18, 2020
@kokoro-team kokoro-team removed the kokoro:run runs the kokoro jobs on a PR label Mar 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

skaffold labels erroneously added to CRD validation schema metadata field
4 participants