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

skip manifest validation for default kubectl deployer #6294

Merged
merged 1 commit into from
Jul 27, 2021

Conversation

gsquared94
Copy link
Contributor

Fixes: #6254

Description

In #6043 we introduced validation for the kubectl deployer manifests. This breaks backward compatibility for users who were using skaffold run without any deployer defined. Although that doesn't seem like a valid usecase for the run command, for versions <= v1.26.0, skaffold run command worked the same as skaffold build command in the absense of a deployer stanza in the skaffold config.

The default deployer (./k8s/*) gets added when no explicit deployer is defined, which now fails the validation introduced in #6043.

To keep backward compat behavior, this PR skips the manifest validations when only the default kubectl deployer is defined.

@gsquared94 gsquared94 requested a review from a team as a code owner July 26, 2021 18:01
@gsquared94 gsquared94 requested a review from aaron-prindle July 26, 2021 18:01
@google-cla google-cla bot added the cla: yes label Jul 26, 2021
@codecov
Copy link

codecov bot commented Jul 26, 2021

Codecov Report

Merging #6294 (7a1a8a6) into main (533016e) will decrease coverage by 0.06%.
The diff coverage is 25.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6294      +/-   ##
==========================================
- Coverage   70.86%   70.79%   -0.07%     
==========================================
  Files         490      494       +4     
  Lines       22162    22334     +172     
==========================================
+ Hits        15704    15811     +107     
- Misses       5441     5503      +62     
- Partials     1017     1020       +3     
Impacted Files Coverage Δ
pkg/skaffold/schema/validation/validation.go 86.20% <25.00%> (-0.70%) ⬇️
pkg/skaffold/docker/image.go 66.55% <0.00%> (-2.33%) ⬇️
pkg/skaffold/docker/parse.go 87.39% <0.00%> (-0.85%) ⬇️
pkg/skaffold/deploy/docker/deploy.go 0.00% <0.00%> (ø)
pkg/skaffold/docker/logger/log.go 23.07% <0.00%> (ø)
pkg/skaffold/docker/tracker/tracker.go 85.29% <0.00%> (ø)
pkg/skaffold/event/v2/debugging.go 100.00% <0.00%> (ø)
pkg/skaffold/docker/logger/formatter.go 75.00% <0.00%> (ø)
...skaffold/kubernetes/debugging/container_manager.go 60.31% <0.00%> (+1.30%) ⬆️
pkg/skaffold/event/v2/event.go 83.79% <0.00%> (+3.79%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 533016e...7a1a8a6. Read the comment docs.

@gsquared94 gsquared94 added the kokoro:force-run forces a kokoro re-run on a PR label Jul 27, 2021
@kokoro-team kokoro-team removed the kokoro:force-run forces a kokoro re-run on a PR label Jul 27, 2021
Copy link
Contributor

@aaron-prindle aaron-prindle left a comment

Choose a reason for hiding this comment

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

LGTM

@gsquared94 gsquared94 merged commit cef5469 into GoogleContainerTools:main Jul 27, 2021
halvards pushed a commit to halvards/skaffold that referenced this pull request Jul 28, 2021
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.

invalid skaffold config with v1.27.0
3 participants