fix: properly add RemoteManifests support to skaffold v2

[aaron-prindle]

fixes #7990

MERGE_AFTER= #8034

[container-tools-bot]

Please visit http://34.66.45.107:1313 to view changes to the docs.

[container-tools-bot]

Error creating deployment, please see controller logs for details.

[codecov]

Codecov Report

Merging #8018 (31438ce) into main (290280e) will decrease coverage by 3.76%.
The diff coverage is 54.09%.

❗ Current head 31438ce differs from pull request most recent head 7f23ef2. Consider uploading reports for the commit 7f23ef2 to get more accurate results

@@            Coverage Diff             @@
##             main    #8018      +/-   ##
==========================================
- Coverage   70.48%   66.71%   -3.77%     
==========================================
  Files         515      596      +81     
  Lines       23150    28899    +5749     
==========================================
+ Hits        16317    19281    +2964     
- Misses       5776     8197    +2421     
- Partials     1057     1421     +364     

Impacted Files	Coverage Δ
cmd/skaffold/app/cmd/credits/export.go	0.00% <0.00%> (ø)
cmd/skaffold/app/cmd/deploy.go	40.90% <0.00%> (-12.94%)	⬇️
cmd/skaffold/app/cmd/test.go	44.44% <0.00%> (ø)
cmd/skaffold/app/exitcode.go	100.00% <ø> (+6.66%)	⬆️
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/render.go	35.48% <18.18%> (-5.90%)	⬇️
cmd/skaffold/app/cmd/lsp.go	28.12% <28.12%> (ø)
cmd/skaffold/app/cmd/run.go	64.28% <33.33%> (-9.63%)	⬇️
cmd/skaffold/app/cmd/fix.go	56.41% <37.50%> (-20.07%)	⬇️
... and 391 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[ericzzzzzzz]

#7990 (comment)
We have some logic to deduce url manifests, not remoteManifests.
In v1 remoteManifests are resources in cluster, not urls where store manifests. implementation is here https://github.com/GoogleContainerTools/skaffold/blob/v1/pkg/skaffold/deploy/kubectl/kubectl.go#L379-L397

we call kubectl get --namespace ${namespace} ${resource name} -o=yaml to retrieve the manifests from clusters.

So I don't think the current change is the right fix, we're currently just not supporting remoteManifests in v2, this change may help supporting reading get manifests from kubenetes resources in remote clusters, but the implementation is hard.

like the reproduction project : https://github.com/rishka/skaffold-manifests-bug/blob/main/skaffold.yaml

the path can just be deploy/test, it's hard to tell if is a local path or some remote k8s resource.

Also, we will need to specify cluster context for this kind of kubectl call, in v1 this readRemoteManifests is a function for deployer, we have a place to specify that kube context in config, but in v2 we cannot specify this kube context under rawK8, we may need a separate config to support reading remote manifests from clusters. Then it's clear that the path is for k8s resources, maybe we don't even have to name it as path, also we're able to specify context. What do you think?

[aaron-prindle]

@ericzzzzzzz thanks for the clarification, I misunderstood the remoteManifests field. I will close this PR and add your comment to the original issue