demonstrate that remoteManifests rendering is broken

[ctrahey]

Related: #5855 #5216

Description

The method ReplaceRemoteManifestImages is no longer used anywhere except the test case modified in this PR. This is a hint that all manifests, even those from remote, go through ReplaceImages. So this one-line PR exists to demonstrate that a regression has taken place for remoteManifests by showing that it breaks a test case; but is correct to be broken, because the without the change in this PR the test is exercising an orphaned code path. The expectations outlined in the test case are real expectations that the community relies on for remote manifests.

If I were a better Go programmer, I would love to also PR a fix for this, but I hope this PR make the issue easy to understand.

TL;DR: If this test case were an integration test rather than a unit test, I think it would have started failing 2.5 years ago.

User facing changes

Before: All tests pass, so we think there is no issue After: kubernetes/manifest unit tests fail, revealing the truth about how remoteManifests no longer work

Follow-up Work (remove if N/A) The handling of remoteManifests needs some tune-up. Commits in #7603 and #7658 fully removed any actual code paths to ReplaceRemoteManifestImages.

[ChrisGe4]

Are you trying to use this PR to notify us the ReplaceRemoteManifestImages or ReplaceImages is not working properly? Or is there a specific test case in TestReplaceRemoteManifestImages should be in TestReplaceImages?

I'm having a hard time to understand the change, and the change doesn't seem to be reasonable to me.

[ctrahey]

Its a bit more nuanced than just a notify-via-pr... The test case I modified here was testing a code path that is not longer reached from anywhere. So the test case was a false positive, but even worse, the expectation that the test case exists to validate is still important - so the fact that my change makes it fail is, in my suggestion, to be taken the same way it should have been taken if the test had started failing back when #7603 and #7658 were open. If they had failed during that work, a fix would likely have been supplied.

[ctrahey]

If it helps clarify, this is my super ugly hack, but this is an absolute pain to maintain across many services:

manifests:
  hooks:
    # This is a hack to work around a defect in Skaffold's rendering of remoteManifests.
    # We're just removing sha digest from the existing deployment because skaffold
    # is accidentally treating remoteManifests the same way as local, where images
    # with digests specified are intentionally skipped.
    before:
      - host:
          command: ["sh", "-c", 'kubectl get deployment -n my-ns -o yaml my-deployment | sed -E "s/^([[:space:]]*image: ${SKAFFOLD_IMAGE_REPO}[^@]*)@sha256.*$/\1/" | kubectl apply -n my-ns --server-side=true --overwrite=true -f -; sleep 5']
          os: [darwin, linux]