Adding "main:" to .ko.yaml causes build to succeed with missing environment variables

[zikaeroh]

This is a re-report from reddit.

I have this as my config:

defaultBaseImage: gcr.io/distroless/base:nonroot
builds:
  - id: main
    ldflags:
      - -X github.com/hortbot/hortbot/internal/version.version={{.Env.HB_VERSION}}

When I run ko, I get an error as expected:

$ ko publish --local --preserve-import-paths .
2021/10/19 18:36:14 Using base gcr.io/distroless/base:nonroot for github.com/hortbot/hortbot
Error: failed to publish images: error building "ko://github.com/hortbot/hortbot": template: argsTmpl:1:61: executing "argsTmpl" at <.Env.HB_VERSION>: map has no entry for key "HB_VERSION"
2021/10/19 18:36:14 error during command execution:failed to publish images: error building "ko://github.com/hortbot/hortbot": template: argsTmpl:1:61: executing "argsTmpl" at <.Env.HB_VERSION>: map has no entry for key "HB_VERSION"

But, change the config to this (adding main:):

defaultBaseImage: gcr.io/distroless/base:nonroot
builds:
  - id: main
    main: github.com/hortbot/hortbot
    ldflags:
      - -X github.com/hortbot/hortbot/internal/version.version={{.Env.HB_VERSION}}

And now it builds, ignoring the missing variable:

$ ko publish --local --preserve-import-paths .
2021/10/19 18:36:21 Using base gcr.io/distroless/base:nonroot for github.com/hortbot/hortbot
2021/10/19 18:36:22 Building github.com/hortbot/hortbot for linux/amd64
2021/10/19 18:36:24 Loading ko.local/github.com/hortbot/hortbot:c8b7f4f7d18e62e542dd1206b7aaa250cae4823dcd47de90ee2e1c9d0743ee55
2021/10/19 18:36:24 Loaded ko.local/github.com/hortbot/hortbot:c8b7f4f7d18e62e542dd1206b7aaa250cae4823dcd47de90ee2e1c9d0743ee55
2021/10/19 18:36:24 Adding tag latest
2021/10/19 18:36:24 Added tag latest
ko.local/github.com/hortbot/hortbot:c8b7f4f7d18e62e542dd1206b7aaa250cae4823dcd47de90ee2e1c9d0743ee55
$ docker run ko.local/github.com/hortbot/hortbot version                               
devel

Adding main: . doesn't hit this (presumably because it's the default).

Perhaps it's intended to be an error to specify a full path?

[imjasonh]

cc @HeavyWombat in case he knows what might be causing this

[imjasonh]

Perhaps it's intended to be an error to specify a full path?

I think that's the case, yeah. I've tested this with the ./test/ package in this repo, with main: ./test/ (see #480) and it works as expected, even with {{ .Env.FOO }} placeholders.

So I think main: . is the expected value in your case.

In the docs for goreleaser (whose config we're borrowing from here) it says:

    # Path to main.go file or main package.

So it sounds like this is supposed to be a file path, and not a full Go importpath. I wonder if we could detect the importpath case and fail with a better descriptive error message, instead of just silently proceeding. 🤔

[zikaeroh]

Presumably, this could be a general check that just verifies that the specified directory or file actually exists; that would have caught my mistake. I don't think it has to be an import path specific case (since reasonably I could write test and that points to a directory named test).

Though, I don't exactly know how this led to the environment variable not working!

[HeavyWombat]

Sorry for the late reply. I will have a look, too. If I am not mistaken, both the dir and the main field should have a default. For main, it could be that . should be the default. The sections in the README tried to explain that, but maybe this could be improved to be more clear.

[HeavyWombat]

Had a look now and I totally misunderstood the issue in the beginning. With main set to an import path, it "only" runs through like it does because it does not match the current build with any of the build configs and therefore builds with an empty default build config in which no environment variables are processed. Therefore, it just runs through.

My two key takeaways here are:

• The error message could be better in case an environment variable is specified that is not set.
• When a build configuration is used, there should be a check that dir plus main actually point to a local file. This would include improving the docs.

[zikaeroh]

I'm not sure I follow on the first point. The error message is fine when it works (map has no entry for key "HB_VERSION"), it's just that when I misconfigured main, that check was seemingly never performed, so I didn't know that I hadn't properly set HB_VERSION until I had already published and went to use my image.