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

[blocked] Skip updates when only provider auth changes #986

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions examples/examples_nodejs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,10 +192,10 @@ func TestSecretsInExplicitProviderNode(t *testing.T) {
})
}
test := getJsOptions(t).With(integration.ProgramTestOptions{
Dir: path.Join(getCwd(t), "test-secrets-in-explicit-provider", "ts"),
Quick: true,
SkipRefresh: true,
ExtraRuntimeValidation: check,
Dir: path.Join(getCwd(t), "test-secrets-in-explicit-provider", "ts"),
AllowEmptyPreviewChanges: false,
SkipEmptyPreviewUpdate: false,
ExtraRuntimeValidation: check,
})
integration.ProgramTest(t, &test)
}
Expand Down
50 changes: 33 additions & 17 deletions examples/test-secrets-in-explicit-provider/ts/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,32 +2,48 @@ import * as pulumi from "@pulumi/pulumi";
import * as docker from "@pulumi/docker";
import * as random from "@pulumi/random";

const providerWithSecretAddress = new docker.Provider("provider-with-sensitive-address", {
registryAuth: [{
const providerWithSecretAddress = new docker.Provider(
"provider-with-sensitive-address",
{
registryAuth: [
{
address: pulumi.secret("secret-address"),
username: "some-user",
}],
})
},
],
}
);

const passwordResource = new random.RandomPassword("random", {
length: 16,
special: false,
length: 16,
special: false,
});

const providerWithSecretUsername = new docker.Provider("provider-with-sensitive-username", {
registryAuth: [{
const providerWithSecretUsername = new docker.Provider(
"provider-with-sensitive-username",
{
registryAuth: [
{
address: "some-address",
username: passwordResource.result,
}],
})
},
],
}
);

const providerWithSecretPassword = new docker.Provider("provider-with-password", {
registryAuth: [{
const providerWithSecretPassword = new docker.Provider(
"provider-with-password",
{
registryAuth: [
{
address: "some-address",
username: "some-user",
password: "secret-password",
}],
})
password: "secret-password-" + Math.random().toString(36).slice(2, 7),
},
],
}
);

export const password = pulumi.unsecret(passwordResource.result)
.apply(x => Buffer.from(x).toString('base64'));
export const password = pulumi
.unsecret(passwordResource.result)
.apply((x) => Buffer.from(x).toString("base64"));
3 changes: 1 addition & 2 deletions provider/hybrid.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,7 @@ func (dp dockerHybridProvider) CheckConfig(ctx context.Context, request *rpc.Che
}

func (dp dockerHybridProvider) DiffConfig(ctx context.Context, request *rpc.DiffRequest) (*rpc.DiffResponse, error) {
// Delegate to the bridged provider, as native Provider does not implement it.
return dp.bridgedProvider.DiffConfig(ctx, request)
return dp.nativeProvider.DiffConfig(ctx, request)
}

func (dp dockerHybridProvider) Configure(
Expand Down
51 changes: 39 additions & 12 deletions provider/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func (p *dockerNativeProvider) CheckConfig(ctx context.Context, req *rpc.CheckRe

// DiffConfig diffs the configuration for this provider.
func (p *dockerNativeProvider) DiffConfig(ctx context.Context, req *rpc.DiffRequest) (*rpc.DiffResponse, error) {
return &rpc.DiffResponse{}, nil
return p.Diff(ctx, req)
}

// Configure configures the resource provider with "globals" that control its behavior.
Expand Down Expand Up @@ -338,27 +338,52 @@ func (p *dockerNativeProvider) Diff(ctx context.Context, req *rpc.DiffRequest) (

func diffUpdates(updates map[resource.PropertyKey]resource.ValueDiff) map[string]*rpc.PropertyDiff {
updateDiff := map[string]*rpc.PropertyDiff{}
for key, valueDiff := range updates {
update := true

if string(key) == "registry" && valueDiff.Object != nil {
// only register a diff on "server" field, but not on "username" or "password",
// as they can change frequently and should not trigger a rebuild.
_, update = valueDiff.Object.Updates["server"]
for key, valueDiff := range updates {
// Include all the same updates by default.
updateDiff[string(key)] = &rpc.PropertyDiff{
Kind: rpc.PropertyDiff_UPDATE,
}

if update {
updateDiff[string(key)] = &rpc.PropertyDiff{
Kind: rpc.PropertyDiff_UPDATE,
// only register a diff on "server" field (or "address" in the case
// of provider config), but not on "username" or "password", as
// they can change frequently and should not trigger a rebuild.
if !(string(key) == "registry" || string(key) == "registryAuth") {
continue
}
keep := false
updates := []*resource.ObjectDiff{}
if valueDiff.Object != nil {
// Resource config.
updates = append(updates, valueDiff.Object)
} else if valueDiff.Array != nil {
// Provider config.
for _, u := range valueDiff.Array.Updates {
updates = append(updates, u.Object)
}
} else if valueDiff.Old.Diff(valueDiff.New) != nil {
continue
}
// Check each modified resource for server/address changes. If we don't
// find any, don't mark this property for update.
for _, u := range updates {
_, serverUpdate := u.Updates["server"]
_, addressUpdate := u.Updates["address"]
if serverUpdate || addressUpdate || u.Updates == nil {
keep = true
break
}
}
if !keep {
delete(updateDiff, string(key))
}
}

return updateDiff
}

// Create allocates a new instance of the provided resource and returns its unique ID afterwards.
func (p *dockerNativeProvider) Create(ctx context.Context, req *rpc.CreateRequest) (*rpc.CreateResponse, error) {

urn := resource.URN(req.GetUrn())
label := fmt.Sprintf("%s.Create(%s)", p.name, urn)
logging.V(9).Infof("%s executing", label)
Expand Down Expand Up @@ -548,7 +573,9 @@ func parseCheckpointObject(obj resource.PropertyMap) resource.PropertyMap {

}

return nil
// If the map doesn't include __inputs then it already represents its
// inputs, as is the case with provider config.
return obj
}

type contextHashAccumulator struct {
Expand Down
72 changes: 72 additions & 0 deletions provider/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,78 @@ func TestDiffUpdates(t *testing.T) {
assert.Equal(t, expected, actual)
})

t.Run("DiffConfig happens on changed address name", func(t *testing.T) {
expected := map[string]*rpc.PropertyDiff{
"registryAuth": {
Kind: rpc.PropertyDiff_UPDATE,
},
}
input := map[resource.PropertyKey]resource.ValueDiff{
"registryAuth": {
Array: &resource.ArrayDiff{
Updates: map[int]resource.ValueDiff{
0: {
Object: &resource.ObjectDiff{
Updates: map[resource.PropertyKey]resource.ValueDiff{
"address": {
Old: resource.PropertyValue{
V: "dockerhub",
},
New: resource.PropertyValue{
V: "ShinyPrivateGHCR",
},
},
},
},
},
},
},
},
}
actual := diffUpdates(input)
assert.Equal(t, expected, actual)
})

t.Run("No DiffConfig happens on changed password", func(t *testing.T) {
expected := map[string]*rpc.PropertyDiff{}
input := map[resource.PropertyKey]resource.ValueDiff{
"registryAuth": {
Array: &resource.ArrayDiff{
Updates: map[int]resource.ValueDiff{
0: {
Object: &resource.ObjectDiff{
Updates: map[resource.PropertyKey]resource.ValueDiff{
"password": {
Old: resource.PropertyValue{
V: "platypus",
},
New: resource.PropertyValue{
V: "Schnabeltier",
},
},
},
},
},
},
},
},
}
actual := diffUpdates(input)
assert.Equal(t, expected, actual)
})

t.Run("No DiffConfig happens on no changes", func(t *testing.T) {
expected := map[string]*rpc.PropertyDiff{}
input := map[resource.PropertyKey]resource.ValueDiff{
"registryAuth": {
Old: resource.NewObjectProperty(resource.PropertyMap{}),
New: resource.NewObjectProperty(resource.PropertyMap{}),
},
}
actual := diffUpdates(input)
assert.Equal(t, expected, actual)
})

t.Run("Diff happens on unknown new registry", func(t *testing.T) {
expected := map[string]*rpc.PropertyDiff{
"registry": {
Expand Down
Loading