decouple helm deployer chartPath into chartPath and remoteChart

[gsquared94]

Fixes: #5459

---

Description

Currently the helm deployer's chartPath property can be used to specify the path to a local Helm chart (packaged or unpacked) or a Helm chart reference (like stable/nginx) or even a URL (like https://example.com/charts/nginx-1.2.3.tgz). It uses the additional remote property to hint skaffold about this.

This PR replaces chartPath and remote properties with explicit chartPath and remoteChart properties.

---

User facing changes (remove if N/A)

Before

deploy:
  helm:
    releases:
    - name: foo1
      chartPath: ./path/to/chart
    - name: foo2
      chartPath: foo2/bar
      remote: true

After

deploy:
  helm:
    releases:
    - name: foo1
      chartPath: ./path/to/chart
    - name: foo2
      remoteChart: foo2/bar

[codecov]

Codecov Report

Merging #5482 (1471947) into master (2e55bec) will increase coverage by 0.21%.
The diff coverage is 80.00%.

@@            Coverage Diff             @@
##           master    #5482      +/-   ##
==========================================
+ Coverage   71.42%   71.64%   +0.21%     
==========================================
  Files         398      399       +1     
  Lines       14654    14749      +95     
==========================================
+ Hits        10467    10567     +100     
+ Misses       3409     3404       -5     
  Partials      778      778              

Impacted Files	Coverage Δ
pkg/skaffold/schema/latest/config.go	58.82% <ø> (ø)
pkg/skaffold/deploy/helm/deploy.go	75.48% <62.50%> (-0.62%)	⬇️
pkg/skaffold/schema/v2beta12/upgrade.go	100.00% <100.00%> (ø)
pkg/skaffold/util/tar.go	50.66% <0.00%> (-5.34%)	⬇️
pkg/skaffold/errors/errors.go	87.23% <0.00%> (ø)
pkg/skaffold/errors/build_problems.go	100.00% <0.00%> (ø)
cmd/skaffold/app/cmd/parse_config.go
pkg/skaffold/parser/config.go	78.68% <0.00%> (ø)
pkg/skaffold/schema/errors/errors.go	100.00% <0.00%> (ø)
cmd/skaffold/app/cmd/runner.go	60.71% <0.00%> (+0.71%)	⬆️
... 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 2e55bec...1471947. Read the comment docs.

[tejal29]

just a nit

[MarlonGamez]

I have a question about this since #5410 just got merged. Would it be possible to keep just the chartPath field, remove the remote field, and basically toggle on/off the remote functionality based on whether or not the user sets the repo field (introduced in #5410)?

[gsquared94]

I have a question about this since #5410 just got merged. Would it be possible to keep just the chartPath field, remove the remote field, and basically toggle on/off the remote functionality based on whether or not the user sets the repo field (introduced in #5410)?

I think that approach won't work because the chartPath can be remote without specifying --repo if the user has already manually pulled it. Also it can be an absolute url in which case the --repo is not required as it is part of that url.

[briandealwis]

remoteChartPath isn't actually a path. Could we try the following:

• keep chartPath for local charts
• remoteChartPath → chartRef or remoteChart?

[MarlonGamez]

LGTM. Do we have anything in examples/ that uses remote helm charts? If not I think that would be a good follow up for this

[gsquared94]

LGTM. Do we have anything in examples/ that uses remote helm charts? If not I think that would be a good follow up for this

Good idea. Added #5517

[zestrells]

Hello @gsquared94 , It looks like when we I try and use the remoteChart variable, it does not work. I verified I am using the Skaffold version 1.21.0

deploy:
  helm:
    releases:
      - name: mysql
        namespace: platcore
        remoteChart: https://gitlab.io/helm-charts/devops/simple-template
        valuesFiles: [ helm/mysql-values.yaml ]

Error I am receiving:
parsing skaffold config: error parsing skaffold configuration file

[briandealwis]

@zestrells try skaffold fix: it provides more detail on the issue. I filed #5659 to improve this error message.

[briandealwis]

Oh and more specifically, that error often indicates that you need to update the version of your skaffold.yaml