Skip to content

Handle nil PortForward item on setting defaults #5416

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

Merged
merged 6 commits into from
Feb 23, 2021

Conversation

Laterality
Copy link
Contributor

Fixes: #5363

Description

Handles default-setting with list of PortForwards containing nil item.

User facing changes (remove if N/A)

When user run with following skaffold.yaml:

apiVersion: skaffold/v2beta10
kind: Config
metadata:
  name: bug
portForward:
 -

Then, will get looks like:

parsing skaffold config: setting default values: Config's portForward[0] is empty, Please check if it has valid values

@Laterality Laterality requested a review from a team as a code owner February 19, 2021 00:28
@google-cla google-cla bot added the cla: yes label Feb 19, 2021
@codecov
Copy link

codecov bot commented Feb 19, 2021

Codecov Report

Merging #5416 (193cce0) into master (4b3c30f) will decrease coverage by 0.22%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5416      +/-   ##
==========================================
- Coverage   71.60%   71.37%   -0.23%     
==========================================
  Files         397      397              
  Lines       14434    14545     +111     
==========================================
+ Hits        10335    10382      +47     
- Misses       3333     3390      +57     
- Partials      766      773       +7     
Impacted Files Coverage Δ
pkg/skaffold/schema/defaults/defaults.go 87.57% <100.00%> (+0.15%) ⬆️
pkg/skaffold/test/structure/structure.go 87.50% <0.00%> (-12.50%) ⬇️
pkg/skaffold/util/tar.go 50.66% <0.00%> (-5.34%) ⬇️
pkg/skaffold/runner/new.go 67.58% <0.00%> (-1.70%) ⬇️
pkg/skaffold/schema/versions.go 82.41% <0.00%> (ø)
pkg/skaffold/test/structure/error.go 100.00% <0.00%> (ø)
pkg/skaffold/test/error.go
pkg/skaffold/test/test.go
pkg/skaffold/test/structure/types.go
pkg/skaffold/schema/v2beta12/upgrade.go 100.00% <0.00%> (ø)
... and 3 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 4b3c30f...193cce0. Read the comment docs.

Copy link
Contributor

@MarlonGamez MarlonGamez left a comment

Choose a reason for hiding this comment

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

Thanks for opening this PR! Have a couple comments but this looks good :)

Copy link
Contributor

@MarlonGamez MarlonGamez left a comment

Choose a reason for hiding this comment

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

This looks good :) Just a small grammar change request

Co-authored-by: Marlon Gamez <[email protected]>
@MarlonGamez MarlonGamez added the kokoro:run runs the kokoro jobs on a PR label Feb 23, 2021
@kokoro-team kokoro-team removed the kokoro:run runs the kokoro jobs on a PR label Feb 23, 2021
@MarlonGamez
Copy link
Contributor

Thanks for making the changes :) Once CI passes I'll approve and merge!

@MarlonGamez MarlonGamez merged commit 33537ea into GoogleContainerTools:master Feb 23, 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.yaml causes skaffold to crash
3 participants