upgrade ko to v6 provider model #69
Conversation
Signed-off-by: Jason Hall <jason@chainguard.dev>
| @@ -41,8 +38,8 @@ func TestAccResourceKoResolve(t *testing.T) { | |||
| } | |||
| `, | |||
| Check: resource.ComposeTestCheckFunc( | |||
| resource.TestMatchResourceAttr("ko_resolve.foo", "manifests.0", regexp.MustCompile("^0: "+url+"/test-46c4b272b3716c422d5ff6dfc7547fa9@sha256:")), | |||
| resource.TestMatchResourceAttr("ko_resolve.foo", "manifests.1", regexp.MustCompile("^1: "+url+"/test-46c4b272b3716c422d5ff6dfc7547fa9@sha256:")), | |||
| resource.TestMatchResourceAttr("ko_resolve.foo", "manifests.0", regexp.MustCompile("0: "+repo.String()+"/github.com/google/ko/test@sha256:")), | |||
There was a problem hiding this comment.
What's driving the namer change? I think we want to match ko's default here so that we work with DockerHub and friends, right?
There was a problem hiding this comment.
I'll back this out, since it's a needless behavior change in an already-too-big change.
But I'd like to get us to the point where we define behaviors like this at the provider level, and we can document "for Dockerhub, set provider's namer to BLAH" and have the default be full importpaths.
I don't think it's worth doing the same for ko since it's harder to configure setting -P for all invocations of ko, but it's easier here in IaC-land.
There was a problem hiding this comment.
ko_build actually defaults to --preserve-import-paths, which is probably not great.
Filed #71 to track aligning these and making a provider-wide default.
Signed-off-by: Jason Hall <jason@chainguard.dev>
| "repo": schema.StringAttribute{ | ||
| Description: "Container repository to publish images to. If set, this overrides the provider's docker_repo, and the image name will be exactly the specified `repo`, without the importpath appended.", | ||
| Optional: true, | ||
| PlanModifiers: []planmodifier.String{stringplanmodifier.RequiresReplace()}, |
There was a problem hiding this comment.
Interesting thought exercise... This is a "ForceNew" but does changing it at the provider level (when it is unspecified here) cause a rebuilt? 🤔
This might be a "ForceNew" hole in our logic, but worth trying out (not sure if a test is practical)
There was a problem hiding this comment.
That's a good point. We can't set plan modifiers on fields at the provider level. Changing it (unless it matched the previous value specified on the resource) would result in a new result with a new ID, but I guess the question is whether TF would understand that as a change that should call Create vs Update. In either case I think we'd be fine, since Update does the same thing as Create. But this is based only on my shaky understanding of TF semantics, and not any kind of testing.
Signed-off-by: Jason Hall <jason@chainguard.dev>
|
Yeah I don't really have any suggestions. There are state upgraders that can move the data from |
|
@imjasonh need rebase |
There are some breaking changes involved here:
ko_imageis removed 🖖docker_repobecomesrepo🧹ko_buildresource'sdocker_repoattribute becomesrepo🧹I expect this to be a rocky upgrade 😕