From 49b69cf94eb7e6b52c9721d69ae530c67984c687 Mon Sep 17 00:00:00 2001 From: Bryce Lampe Date: Thu, 25 Jan 2024 11:36:28 -0800 Subject: [PATCH] implement diff, mutex around auth --- examples/aws-container-registry/ts/index.ts | 21 ++---- .../docker-container-registry/ts/index.ts | 21 ++---- provider/internal/client.go | 30 +++++---- provider/internal/image.go | 66 +++++++++++++++++++ provider/internal/image_test.go | 2 +- 5 files changed, 100 insertions(+), 40 deletions(-) diff --git a/examples/aws-container-registry/ts/index.ts b/examples/aws-container-registry/ts/index.ts index 0acc9cc7..2858dbdc 100644 --- a/examples/aws-container-registry/ts/index.ts +++ b/examples/aws-container-registry/ts/index.ts @@ -46,8 +46,13 @@ export const repoDigest = image.repoDigest; // buildx -const provider = new docker.Provider("docker", { - registryAuth: [ +const buildxImage = new docker.buildx.Image("buildx", { + tags: [pulumi.interpolate`${repo.repositoryUrl}:buildx`], + exports: ["type=registry"], + file: "app/Dockerfile", + platforms: ["linux/arm64", "linux/amd64"], + context: "app", + registries: [ { address: registryInfo.server, username: registryInfo.username, @@ -56,16 +61,4 @@ const provider = new docker.Provider("docker", { ], }); -const buildxImage = new docker.buildx.Image( - "buildx", - { - tags: [pulumi.interpolate`${repo.repositoryUrl}:buildx`], - exports: ["type=registry"], - file: "app/Dockerfile", - platforms: ["linux/arm64", "linux/amd64"], - context: "app", - }, - { provider: provider } -); - export const manifests = buildxImage.manifests; diff --git a/examples/docker-container-registry/ts/index.ts b/examples/docker-container-registry/ts/index.ts index 0edfdf7a..d0454a5b 100644 --- a/examples/docker-container-registry/ts/index.ts +++ b/examples/docker-container-registry/ts/index.ts @@ -29,8 +29,13 @@ export const repoDigest = image.repoDigest; // buildx -const provider = new docker.Provider("docker", { - registryAuth: [ +const buildxImage = new docker.buildx.Image("my-buildx-image", { + tags: [`${imageName}:buildx`], + exports: ["type=registry"], + platforms: ["linux/arm64", "linux/amd64"], + context: "app", + file: "app/Dockerfile", + registries: [ { address: registryInfo.server, username: registryInfo.username, @@ -39,16 +44,4 @@ const provider = new docker.Provider("docker", { ], }); -const buildxImage = new docker.buildx.Image( - "my-buildx-image", - { - tags: [`${imageName}:buildx`], - exports: ["type=registry"], - platforms: ["linux/arm64", "linux/amd64"], - context: "app", - file: "app/Dockerfile", - }, - { provider: provider } -); - export const manifests = buildxImage.manifests; diff --git a/provider/internal/client.go b/provider/internal/client.go index b160d40a..670bc2f4 100644 --- a/provider/internal/client.go +++ b/provider/internal/client.go @@ -6,6 +6,7 @@ import ( "fmt" "os" "strings" + "sync" "github.com/distribution/reference" cbuild "github.com/docker/buildx/controller/build" @@ -36,6 +37,8 @@ type Client interface { } type docker struct { + mu sync.Mutex // Guards changes to configs. + cli *command.DockerCli dir string } @@ -73,6 +76,9 @@ func newDockerClient() (*docker, error) { } func (d *docker) Auth(ctx context.Context, creds properties.RegistryAuth) error { + d.mu.Lock() + defer d.mu.Unlock() + cfg := d.cli.ConfigFile() // Special handling for legacy DockerHub domains. The OCI-compliant @@ -93,17 +99,19 @@ func (d *docker) Auth(ctx context.Context, creds properties.RegistryAuth) error if existing, err := cfg.GetAuthConfig(creds.Address); err == nil { // Confirm the auth is still valid. Otherwise we'll set it to the // provided config. - _, err = d.cli.Client().RegistryLogin(ctx, registrytypes.AuthConfig{ - Auth: existing.Auth, - Email: existing.Email, - IdentityToken: existing.IdentityToken, - Password: existing.Password, - RegistryToken: existing.RegistryToken, - ServerAddress: creds.Address, // ServerAddress is sometimes empty. - Username: existing.Username, - }) - if err == nil { - return nil // Creds still work, nothing to do. + if existing.Username == creds.Username { + _, err = d.cli.Client().RegistryLogin(ctx, registrytypes.AuthConfig{ + Auth: existing.Auth, + Email: existing.Email, + IdentityToken: existing.IdentityToken, + Password: existing.Password, + RegistryToken: existing.RegistryToken, + ServerAddress: creds.Address, // ServerAddress is sometimes empty? + Username: existing.Username, + }) + if err == nil { + return nil // Creds still work, nothing to do. + } } } diff --git a/provider/internal/image.go b/provider/internal/image.go index c86ba886..7d5fdb0e 100644 --- a/provider/internal/image.go +++ b/provider/internal/image.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "reflect" // These imports are needed to register the drivers with buildkit. _ "github.com/docker/buildx/driver/docker-container" @@ -32,6 +33,7 @@ var ( _ infer.Annotated = (infer.Annotated)((*ImageState)(nil)) _ infer.CustomCheck[ImageArgs] = (*Image)(nil) _ infer.CustomDelete[ImageState] = (*Image)(nil) + _ infer.CustomDiff[ImageArgs, ImageState] = (*Image)(nil) _ infer.CustomRead[ImageArgs, ImageState] = (*Image)(nil) _ infer.CustomResource[ImageArgs, ImageState] = (*Image)(nil) ) @@ -148,6 +150,10 @@ func (*Image) Check( // we're authenticated in almost all cases. cfg := infer.GetConfig[Config](ctx) for _, reg := range args.Registries { + // TODO(https://github.com/pulumi/pulumi-go-provider/pull/155): This is likely unresolved. + if reg.Address == "" { + continue + } if err = cfg.client.Auth(ctx, reg); err != nil { failures = append(failures, provider.CheckFailure{Property: "registries", Reason: fmt.Sprintf("unable to authenticate: %s", err.Error())}) @@ -192,6 +198,10 @@ func (ia *ImageArgs) toBuildOptions() (controllerapi.BuildOptions, error) { } for _, t := range ia.Tags { + if t == "" { + // TODO(https://github.com/pulumi/pulumi-go-provider/pull/155): This is likely unresolved. + continue + } if _, err := reference.Parse(t); err != nil { multierr = errors.Join(multierr, newCheckFailure("tags", err)) } @@ -366,6 +376,62 @@ func (*Image) Delete( return err } +// Diff re-implements most of the default diff behavior, with the exception of +// ignoring "password" changes on registry inputs. +func (*Image) Diff(_ provider.Context, id string, olds ImageState, news ImageArgs) (provider.DiffResponse, error) { + diff := map[string]provider.PropertyDiff{} + update := provider.PropertyDiff{Kind: provider.UpdateReplace} // TODO: Implement Update. + + if !reflect.DeepEqual(olds.BuildArgs, news.BuildArgs) { + diff["buildArgs"] = update + } + if !reflect.DeepEqual(olds.CacheFrom, news.CacheFrom) { + diff["cacheFrom"] = update + } + if !reflect.DeepEqual(olds.CacheTo, news.CacheTo) { + diff["cacheTo"] = update + } + if olds.Context != news.Context { + diff["context"] = update + } + if !reflect.DeepEqual(olds.Exports, news.Exports) { + diff["exports"] = update + } + if olds.File != news.File { + diff["file"] = update + } + if !reflect.DeepEqual(olds.Platforms, news.Platforms) { + diff["platforms"] = update + } + if olds.Pull != news.Pull { + diff["pull"] = update + } + if !reflect.DeepEqual(olds.Tags, news.Tags) { + diff["tags"] = update + } + + // Registries need special handling because we ignore "password" changes to not introduce unnecessary changes. + if len(olds.Registries) != len(news.Registries) { + diff["registries"] = update + } else { + for idx, oldr := range olds.Registries { + newr := news.Registries[idx] + if (oldr.Username == newr.Username) && (oldr.Address == newr.Address) { + continue + } + diff["registries"] = update + break + } + } + + return provider.DiffResponse{ + DeleteBeforeReplace: false, + HasChanges: len(diff) > 0, + DetailedDiff: diff, + }, nil +} + +// Cancel cleans up temporary on-disk credentials. func (*Image) Cancel(ctx provider.Context) error { cfg := infer.GetConfig[Config](ctx) return cfg.client.Close(ctx) diff --git a/provider/internal/image_test.go b/provider/internal/image_test.go index fde698c9..bb47ba99 100644 --- a/provider/internal/image_test.go +++ b/provider/internal/image_test.go @@ -86,7 +86,7 @@ func TestLifecycle(t *testing.T) { resource.NewObjectProperty(resource.PropertyMap{ "address": resource.NewStringProperty("fakeaddress"), "username": resource.NewStringProperty("fakeuser"), - "password": resource.NewStringProperty("fakepass"), + "password": resource.MakeSecret(resource.NewStringProperty("password")), }), }, ),