fix(resource): don't use -f when patch file is provided

[agilgur5]

Fixes #13279 per #13279 (reply in thread)
Fixes #11248 and Supersedes #11249

Motivation

When using kubectl patch:

1. If a patch file is provided (not a full manifest), then you have to use -p and provide a kind and name in flags. You cannot use -f in this case.
2. If a full manifest is provided, then you have to use both -p and -f
   • in this form, -f is expected to have a header containing the kind and name (and potentially more)
   • and -p is the patch file
   • in our case, only one manifest is provided that acts as both: a patch file with a header

Modifications

• don't add -f if there are flags and a kind is provided in the manifest
• use --patch-file instead of -p, no need to stringify
  • (--patch-file has existed since at least k8s 1.20. we import the kubectl package though, so it only depends on the version we use in go.mod)
• update unit tests
• update & add examples, which are used as part of the test-examples E2E suite

Verification

Unit tests and E2E tests (the examples are tested)

[agilgur5]

but this could use an E2E, and I don't think one exists yet for resource templates?

Ah, the example tests actually take care of this. Unfortunately they are failing:

Running examples/k8s-patch-basic.yaml...
workflow.argoproj.io/k8s-patch-basic-vfsbq created
error: timed out waiting for the condition on workflows/k8s-patch-basic-vfsbq
NAME                    STATUS    AGE   MESSAGE
k8s-patch-basic-vfsbq   Running   30s   

The error lacks some detail though:

    State:          Terminated
      Reason:       Error
      Exit Code:    1

and there are no logs in CI because only the wait container logs are retrieved, and a resource template has no wait container. Will PR a fix shortly to get logs of all containers in the failure debug, which should be helpful for other purposes as well. EDIT: Fixed in #13318

[agilgur5]

and there are no logs

Here's the error running locally:

time="2024-07-06T22:55:25.552Z" level=info msg="kubectl patch --type strategic --patch-file /tmp/manifest.yaml -o json"
error: You must provide one or more resources by argument or filename.
Example resource specifications include:
   '-f rsrc.yaml'
   '--filename=rsrc.json'
   '<resource> <name>'
   '<resource>'

In other words, --patch-file only works if a resource has been specified. So this does require that flags are specified, meaning that we do have to do some inference to determine whether -f or --patch-file should be used, as they are not equivalent.

Also noting that the json patch example, which has flags, works correctly

[Joibel]

I'd rather this (and the other manifestPaths under test here) were specific to the test suite, not overloading other files which have their own purpose. The effect of changing or deleting one of these (albeit unlikely) and seeing this test failing would be very surprising.

[agilgur5]

I agree, but the existing tests already did that, so I did not change that test behavior in this PR. Changing that seemed to require a more substantial refactor, as we don't have a testdata dir for unit tests (this requires a file to read in) or much use of full manifests in units, so I deferred that for the purposes of this fix PR.

I could make a separate PR for that, but we'd have to think of a strategy to apply more generically to these types of unit tests that a need a testdata dir or similar

[Joibel]

Ok, that's reasonable.