Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement "strict mode" which requires ko:// prefix for import paths #58

Merged
merged 5 commits into from
Aug 16, 2019

Conversation

imjasonh
Copy link
Member

@imjasonh imjasonh commented Jul 10, 2019

Demonstrating a strawman implementation and UX for "strict mode", as described in #9

In strict mode, image references are only considered if they begin with the string ko://. In loose mode, ko:// refs are considered as image references and built and replaced during YAML resolution.

If we decide to pursue this model, we could start by making strictness opt-in, cut a versioned release of ko, then make --strict=true by default (opt-out), then eventually maybe even remove the option.

$ cat test.yaml
looseImage: github.com/google/ko/cmd/ko
strictImage: ko://github.com/google/ko/cmd/ko
$ KO_DOCKER_REPO=gcr.io/foo go run ./cmd/ko/ resolve -f test.yaml
...
looseImage: gcr.io/foo/ko-099ba5bcefdead87f92606265fb99ac0@sha256:dbe1a3e11de9e701c9e7a5e61d4f3c2dc93eee2552bb7f6831a23c1147e6899a
strictImage: gcr.io/foo/ko-099ba5bcefdead87f92606265fb99ac0@sha256:dbe1a3e11de9e701c9e7a5e61d4f3c2dc93eee2552bb7f6831a23c1147e6899a

---
$ KO_DOCKER_REPO=gcr.io/foo go run ./cmd/ko/ resolve -f test.yaml -S
...
looseImage: github.com/google/ko/cmd/ko
strictImage: gcr.io/foo/ko-099ba5bcefdead87f92606265fb99ac0@sha256:dbe1a3e11de9e701c9e7a5e61d4f3c2dc93eee2552bb7f6831a23c1147e6899a
---

Sending this out to get feedback on the overall idea; it sounds like Go modules is going to be pretty painful without this as an option at least.

@imjasonh imjasonh requested a review from jonjohnsonjr July 10, 2019 12:18
@imjasonh imjasonh changed the title WIP support for "strict mode" which requires ko:// prefix for image refs Implement "strict mode" which requires ko:// prefix for import paths Aug 15, 2019
@imjasonh
Copy link
Member Author

imjasonh commented Aug 15, 2019

$ cat test.yaml 
looseImage: github.com/google/ko/cmd/ko          # unresolved in strict mode
strictImage: ko://github.com/google/ko/cmd/ko    # resolved in any mode
badImage: github.com/blargh/whatever             # unresolved in either mode
badStrictImage: ko://github.com/blargh/whatever  # resolved in loose mode; fails in strict mode

# loose mode
$ KO_DOCKER_REPO=gcr.io/my-project go run ./cmd/ko/ resolve -f test.yaml
badImage: github.com/blargh/whatever
badStrictImage: ko://github.com/blargh/whatever
looseImage: gcr.io/my-project/ko-099ba5bcefdead87f92606265fb99ac0@sha256:174089f40c1c0f9f5103c7858a39827ed50b80c6a7b877cf63714b8029b95fcf
strictImage: gcr.io/my-project/ko-099ba5bcefdead87f92606265fb99ac0@sha256:174089f40c1c0f9f5103c7858a39827ed50b80c6a7b877cf63714b8029b95fcf

# strict mode
$ KO_DOCKER_REPO=gcr.io/my-project go run ./cmd/ko/ resolve -S -f test.yaml
2019/08/15 09:42:47 error processing import paths in "test.yaml": Strict reference "ko://github.com/blargh/whatever" is not supported

If you remove badStrictImage the strict-mode resolved output is:

badImage: github.com/blargh/whatever
looseImage: github.com/google/ko/cmd/ko
strictImage: gcr.io/test-argo/ko-099ba5bcefdead87f92606265fb99ac0@sha256:174089f40c1c0f9f5103c7858a39827ed50b80c6a7b877cf63714b8029b95fcf

Non-ko://-prefixed paths are ignored even if they're otherwise supported. Only paths that begin with ko:// are considered in strict mode.

When ko is invoked in this mode, import paths must have the `ko://`
prefix. If a human marks an import path with `ko://` and ko can't
resolve the resulting import path, it fails. In "loose mode", such an
import path would be silently ignored and passed on to the resolved
YAML, often resulting in invalid image names (e.g., `image:
github.com/foo/bar`)

In loose mode, `ko://` prefixes are always ignored for
backward-compatibility.
Strictness has nothing to do with building, and is independent of how
images are built (fixed builder, some future exotic builder type, etc.)
Copy link
Collaborator

@jonjohnsonjr jonjohnsonjr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a couple nits but lgtm

}

func AddStrictArg(cmd *cobra.Command, so *StrictOptions) {
cmd.Flags().BoolVarP(&so.Strict, "strict", "S", so.Strict,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm mildly -1 on including -S -- we will eventually run out of flags if we keep this up :P

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I kind of agree. I'll remove -S until someone complains it's too much to type.

pkg/commands/options/strict.go Outdated Show resolved Hide resolved
@imjasonh imjasonh merged commit b7eb9df into ko-build:master Aug 16, 2019
evandbrown pushed a commit to google/kf that referenced this pull request Jan 8, 2020
To allow for ko and Go modules to work well, we need to use the `ko://`
prefix for our paths. This allows us to use `ko apply --strict ...`.
More information can be found in the ko
[PR](ko-build/ko#58).

Change-Id: I5f33c56465476c81ff73085ccc5776064f8187cf
poy added a commit to poy/pipeline that referenced this pull request Mar 11, 2020
This allows `ko` to improve the experience with Go modules. See
ko-build/ko#58 for more information.
tekton-robot pushed a commit to tektoncd/pipeline that referenced this pull request Apr 1, 2020
This allows `ko` to improve the experience with Go modules. See
ko-build/ko#58 for more information.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants