fix: choose cli default-repo over config file

[tomasmota]

Fixes: #7125

Description
Fixes error introducing in pr #6985. Logic was introduced there to give precedence to config value instead of cli.

[codecov]

Codecov Report

Merging #7144 (7dc6bc4) into main (290280e) will decrease coverage by 2.09%.
The diff coverage is 56.96%.

@@            Coverage Diff             @@
##             main    #7144      +/-   ##
==========================================
- Coverage   70.48%   68.39%   -2.10%     
==========================================
  Files         515      560      +45     
  Lines       23150    26253    +3103     
==========================================
+ Hits        16317    17955    +1638     
- Misses       5776     7061    +1285     
- Partials     1057     1237     +180     

Impacted Files	Coverage Δ
cmd/skaffold/app/cmd/deploy.go	52.00% <ø> (-1.85%)	⬇️
cmd/skaffold/app/cmd/dev.go	84.61% <0.00%> (ø)
cmd/skaffold/app/cmd/render.go	36.66% <0.00%> (-4.72%)	⬇️
cmd/skaffold/skaffold.go	0.00% <0.00%> (ø)
cmd/skaffold/app/cmd/inspect_tests.go	62.50% <14.28%> (-1.14%)	⬇️
cmd/skaffold/app/cmd/lsp.go	28.12% <28.12%> (ø)
cmd/skaffold/app/cmd/fix.go	68.85% <40.00%> (-7.62%)	⬇️
cmd/skaffold/app/cmd/lint.go	42.85% <42.85%> (ø)
cmd/skaffold/app/cmd/find_configs.go	48.88% <50.00%> (+0.24%)	⬆️
cmd/skaffold/app/skaffold.go	76.19% <70.00%> (-8.43%)	⬇️
... and 217 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 1dbc7a2...7dc6bc4. Read the comment docs.

[gsquared94]

is cfg.DefaultRepo being checked somewhere else? Otherwise I think the author intended this to say:

if defaultRepo.Value() == nil && cfg.DefaultRepo != "" {
		defaultRepo = NewStringOrUndefined(&cfg.DefaultRepo)
}

[tomasmota]

If I don't pass in the flag, but have it in my config, it is still being picked up without that piece of code. I would assume that implies that it is checked somewhere else, right?

[MarlonGamez]

the reason I believe this piece of code is not needed is because typically when we call the following function:

skaffold/pkg/skaffold/deploy/util/util.go

Lines 34 to 52 in 1dbc7a2

	// ApplyDefaultRepo applies the default repo to a given image tag.
	func ApplyDefaultRepo(globalConfig string, defaultRepo *string, tag string) (string, error) {
	repo, err := config.GetDefaultRepo(globalConfig, defaultRepo)
	if err != nil {
	return "", fmt.Errorf("getting default repo: %w", err)
	}
	multiLevel, err := config.GetMultiLevelRepo(globalConfig)
	if err != nil {
	return "", fmt.Errorf("getting multi-level repo support: %w", err)
	}
	newTag, err := docker.SubstituteDefaultRepoIntoImage(repo, multiLevel, tag)
	if err != nil {
	return "", fmt.Errorf("applying default repo to %q: %w", tag, err)
	}
	return newTag, nil
	}

We grab the value from the config and compare things then. This is how things usually worked before these few lines were added I believe

[gsquared94]

sounds good

[tomasmota]

Could the PR actually have broken something in that test? I'm not familiar with kaniko, so I'd have a hard time debugging this

[tejal29]

There is an open issue #7121. Not your fault!!

[gsquared94]

lgtm