Refactor global values to be defaults

[nmittler]

There was recent work to add global values for env, flags, and ldflags. The global values would be merged with per-build values to generate the value used for the builds.

There are a couple issues with this:

• It's inconsistent with the existing code, which only has default values declared globally (there is no merging today).
• The name of the flag variable, caused a conflict with knative's KO_FLAGS environment variable (see 0.15.3 has breaking issues error resolving image references: build: go build: exit status 2: flag provided but not defined: -platform #1317)

This PR does the following:

• Refactors the logic to use defaultEnv, defaultFlags, and defaultLdflags. This resolves both issues described above.
• Updates documentation

Fixes #1317

[nmittler]

@dprotaso @imjasonh PTAL

[nmittler]

The integration test I added does seem to indicate that the original error is gone.

However, it fails on my m1 macbook. with:

WARNING: The requested image's platform (linux/amd64) does not match the detected host platform (linux/arm64/v8) and no specific platform was requested
exec /ko-app/test: exec format error

The test should be using the GOARCH of the local platform, but it apparently doesn't. This makes me think it's not actually using the value passed in by KO_FLAGS.

I can't see how my recent changes could be related, however.

[dprotaso]

This makes me think it's not actually using the value passed in by KO_FLAGS.

It's not - so knative uses this in our scripts and we invoke ko like ko resolve ${KO_FLAGS} so it was sorta a coincidence your change started reading that flag.

Hence why I think it might be better to be more descriptive with the flags.

eg. defaultFlags => goBuildFlags then that could map to KO_GO_BUILD_FLAGS or something

[nmittler]

@dprotaso

It's not - so knative uses this in our scripts and we invoke ko like ko resolve ${KO_FLAGS} so it was sorta a coincidence your change started reading that flag.

Ah, that makes sense ... I apparently missed your comment on the issue where you clarified this.

But given that ko is effectively reserving the KO_ prefix through viper, isn't the real issue that knative is using that prefix in the first place? There will always be a chance of collision going forward otherwise.

I'm a little concerned with goBuildXXX names with respect to consistency:

defaultBaseImage: ...
defaultPlatforms:
- linux/arm64
- linux/amd64
goBuildEnv:
- FOO=bar
goBuildFlags:
- -v
goBuildLdflags:
- -s
builds:
- id: foo
  dir: .
  main: ./foobar/foo
  flags:
  - -trimpath
  ldflags:
  - -w
...

This seems to break the naming convention that was established with defaultXXX. No other variable uses go (which I suppose is just assumed since ko only builds go applications) or build (all of the config is effectively build-related).

From the perspective of ko, just sticking with the defaultXXX pattern would seem to make sense both in name and behavior.

Thoughts?

[nmittler]

@imjasonh @cpanato

Any concerns about the breaking changes in this PR? I think regardless which way we go, something will break between 0.15.3 and 0.15.4.

Alternatively, we could just patch the immediate issue by using viper's EnvKeyReplacer. IIUC this would allow us to just manually define the environment variable we use for flags to something like KO_BUILD_FLAGS. I hacked this together in main...nmittler:ko:fix2.

[imjasonh]

I'm comfortable breaking behavior from 0.15.3 -> 0.15.4, since it's very unlikely anybody has come to rely on it during that short window. (If you're here because you came to rely on it during that short window, I'm sorry, I owe you a beer! 🍻 )