Skip to content

Commit

Permalink
Fix #2442 (#3638)
Browse files Browse the repository at this point in the history
Fixes #2442 

This adds a diff customizer that ignores changes to parameters that only
change the apply_method and not the value for the aws_db_parameter_group
resource. To make this work, the change also needs to modify the set
element hashing function to identify parameters that differ only by
apply_method as identical.

As a side-effect of the set hashing change, upgrading stacks to the
newer version of the provider with this change will show a reordering
update diff on the ParameterGroup resource.
  • Loading branch information
t0yv0 authored Mar 19, 2024
1 parent 385fe01 commit caf7ae5
Show file tree
Hide file tree
Showing 12 changed files with 336 additions and 40 deletions.
13 changes: 11 additions & 2 deletions provider/cmd/pulumi-resource-aws/schema.json

Large diffs are not rendered by default.

123 changes: 123 additions & 0 deletions provider/pkg/rds/parameter_group.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
// Copyright 2016-2024, Pulumi Corporation.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package rds

import (
"context"
"fmt"
"os"
"path/filepath"
"regexp"
"strings"

"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"

"github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge"
)

var (
parameterGroupApplyMethodRegexp = regexp.MustCompile(`^parameter\.(\d+)\.apply_method$`)
parameterGroupNote = strings.TrimSpace(strings.ReplaceAll(`
**NOTE**: to make diffs less confusing, the AWS provider will ignore changes for a 'parameter' whose 'value' remains
unchanged but whose 'apply_method' is changing (e.g., from 'immediate' to 'pending-reboot', or 'pending-reboot' to
'immediate'). This matches the cloud: if only the apply method of a parameter is changing, the AWS API will not register
this change. To change the 'apply_method' of a parameter, its value must also change.
`, "'", "`"))
)

func ParameterGroupDocs(upstreamPath string) *tfbridge.DocInfo {
relPath := "website/docs/r/db_parameter_group.html.markdown"
fp := filepath.Join(append([]string{upstreamPath}, strings.Split(relPath, "/")...)...)
bytes, err := os.ReadFile(fp)
if err != nil {
// at runtime the file may not be available, bail
return nil
}
pattern := regexp.MustCompile(
"(?s)[*][*]NOTE:[*][*] After applying your changes, (.*?) must also change.",
)
newNote := []byte(parameterGroupNote)
markdown := pattern.ReplaceAll(bytes, newNote)
return &tfbridge.DocInfo{
Markdown: markdown,
}
}

// Customize the aws_db_parameter_group resource.
func parameterGroupReconfigure(p *schema.Provider) {
parameterGroup, ok := p.ResourcesMap["aws_db_parameter_group"]
if !ok {
return
}
// Need to mark apply_method as Computed so that the diff customizer is allowed to clear it.
parameterGroup.Schema["parameter"].Elem.(*schema.Resource).Schema["apply_method"].Computed = true
// Need to exclude apply_method from the set hash.
oldSetFunc := parameterGroup.Schema["parameter"].Set
parameterGroup.Schema["parameter"].Set = parameterGroupParameterSetFunc(oldSetFunc)
addDiffCustomizer(parameterGroup, parameterGroupCustomizeDiff)
}

// Exclude "apply_method" from influencing the set element hash for a parameter.
func parameterGroupParameterSetFunc(oldSetFunc schema.SchemaSetFunc) schema.SchemaSetFunc {
return func(v interface{}) int {
m := v.(map[string]interface{})
// Pretend apply_method is always "immediate" to avoid changing the hash
copy := make(map[string]interface{}, len(m))
for k, v := range m {
copy[k] = v
}
copy["apply_method"] = "immediate"
return oldSetFunc(copy)
}
}

// Customize the diff to pretend that no changes are happening for a parameter that changes its apply_method but not its
// value. This makes the provider match cloud behavior better.
func parameterGroupCustomizeDiff(ctx context.Context, diff *schema.ResourceDiff, meta interface{}) error {
// If the apply_method is changed, and the value is not, then clear the apply_method change to ignore it.
for _, changedKey := range diff.GetChangedKeysPrefix("parameter") {
// Surprisingly GetChangedKeysPrefix returns keys that did not actually change, so skip those.
if !diff.HasChange(changedKey) {
continue
}
if m := parameterGroupApplyMethodRegexp.FindStringSubmatch(changedKey); m != nil {
parameterIndex := m[1]
matchingValueKey := fmt.Sprintf("parameter.%s.value", parameterIndex)
if !diff.HasChange(matchingValueKey) {
if err := diff.Clear(changedKey); err != nil {
return fmt.Errorf("Failed clearing %s: %w", changedKey, err)
}
}
}
}
return nil
}

// This could have been a helper function in pulumi-terraform-bridge somewhere.
func addDiffCustomizer(r *schema.Resource, cdf schema.CustomizeDiffFunc) {
// Just set it if it is not yet customized.
if r.CustomizeDiff == nil {
r.CustomizeDiff = cdf
return
}
// Sequence to compose the custom diff functions otherwise.
oldCDF := r.CustomizeDiff
r.CustomizeDiff = func(ctx context.Context, diff *schema.ResourceDiff, meta interface{}) error {
if err := oldCDF(ctx, diff, meta); err != nil {
return err
}
return cdf(ctx, diff, meta)
}
}
24 changes: 24 additions & 0 deletions provider/pkg/rds/rds.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Copyright 2016-2024, Pulumi Corporation.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package rds

import (
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
)

// Customize resources in the rds module.
func ReconfigureResources(p *schema.Provider) {
parameterGroupReconfigure(p)
}
102 changes: 102 additions & 0 deletions provider/provider_yaml_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,21 @@
package provider

import (
"fmt"
"math/rand"
"os"
"path/filepath"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/pulumi/pulumi/sdk/v3/go/common/apitype"

"github.com/pulumi/providertest"
"github.com/pulumi/providertest/pulumitest"
"github.com/pulumi/providertest/pulumitest/assertpreview"
"github.com/pulumi/providertest/pulumitest/opttest"
)

func TestBucket(t *testing.T) {
Expand Down Expand Up @@ -126,3 +137,94 @@ func TestCloudfrontDistribution(t *testing.T) {
func TestSecretVersion(t *testing.T) {
test(t, filepath.Join("test-programs", "secretversion"), "")
}

func TestRdsParameterGroupUnclearDiff(t *testing.T) {
if testing.Short() {
t.Skipf("Skipping in testing.Short() mode, assuming this is a CI run without credentials")
}

type testCase struct {
name string
applyMethod1 string
value1 string
applyMethod2 string
value2 string
expectChanges bool
}

testCases := []testCase{
{"changing only applyMethod", "immediate", "1", "pending-reboot", "1", false},
{"changing only value", "immediate", "1", "immediate", "0", true},
{"changing both applyMethod and value", "immediate", "1", "pending-reboot", "0", true},
}

cwd, err := os.Getwd()
require.NoError(t, err)

yaml := `
name: project
runtime: yaml
config:
applyMethod:
type: string
value:
type: string
randSuffix:
type: string
resources:
default:
type: aws:rds/parameterGroup:ParameterGroup
properties:
name: securitygroup${randSuffix}
family: postgres14
parameters:
- name: track_io_timing
value: ${value}
applyMethod: ${applyMethod}
- name: "log_checkpoints"
applyMethod: "pending-reboot"
value: "1"
`

for _, tc := range testCases {
tc := tc

t.Run(tc.name, func(t *testing.T) {
workdir := t.TempDir()

err := os.WriteFile(filepath.Join(workdir, "Pulumi.yaml"), []byte(yaml), 0600)
require.NoError(t, err)

pt := pulumitest.NewPulumiTest(t, workdir,
opttest.SkipInstall(),
opttest.TestInPlace(),
opttest.LocalProviderPath("aws", filepath.Join(cwd, "..", "bin")),
)

pt.SetConfig("randSuffix", fmt.Sprintf("%d", rand.Intn(1024*1024)))

pt.SetConfig("applyMethod", tc.applyMethod1)
pt.SetConfig("value", tc.value1)

pt.Up()

assertpreview.HasNoChanges(t, pt.Preview())

pt.SetConfig("applyMethod", tc.applyMethod2)
pt.SetConfig("value", tc.value2)

if tc.expectChanges {
pr := pt.Preview()
assert.Equal(t, 1, pr.ChangeSummary[apitype.OpUpdate])
} else {
assertpreview.HasNoChanges(t, pt.Preview())
}

upr := pt.Up()
t.Logf("stdout: %s", upr.StdOut)
t.Logf("stderr: %s", upr.StdErr)

assertpreview.HasNoChanges(t, pt.Preview())
})
}
}
9 changes: 2 additions & 7 deletions provider/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ import (
"github.com/aws/aws-sdk-go-v2/feature/ec2/imds"
awsbase "github.com/hashicorp/aws-sdk-go-base/v2"
tfschema "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
awsShim "github.com/hashicorp/terraform-provider-aws/shim"
"github.com/mitchellh/go-homedir"
"github.com/pulumi/pulumi-aws/provider/v6/pkg/rds"
"github.com/pulumi/pulumi-aws/provider/v6/pkg/version"

pftfbridge "github.com/pulumi/pulumi-terraform-bridge/pf/tfbridge"
Expand Down Expand Up @@ -780,12 +780,6 @@ func Provider() *tfbridge.ProviderInfo {
return ProviderFromMeta(tfbridge.NewProviderMetadata(runtimeMetadata))
}

func newUpstreamProvider(ctx context.Context) awsShim.UpstreamProvider {
upstreamProvider, err := awsShim.NewUpstreamProvider(ctx)
contract.AssertNoErrorf(err, "NewUpstreamProvider failed to initialize")
return upstreamProvider
}

func deprecateRuntime(value, name string) schema.EnumValueSpec {
s := schema.EnumValueSpec{Value: value, Name: name}
s.DeprecationMessage = "This runtime is now deprecated"
Expand Down Expand Up @@ -3102,6 +3096,7 @@ func ProviderFromMeta(metaInfo *tfbridge.MetadataInfo) *tfbridge.ProviderInfo {
Default: managedByPulumi,
},
},
Docs: rds.ParameterGroupDocs("upstream"),
},
"aws_db_instance_role_association": {
Tok: awsResource(rdsMod, "RoleAssociation"),
Expand Down
37 changes: 37 additions & 0 deletions provider/upstream_provider.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// Copyright 2016-2024, Pulumi Corporation.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package provider

import (
"context"

awsShim "github.com/hashicorp/terraform-provider-aws/shim"
"github.com/pulumi/pulumi-aws/provider/v6/pkg/rds"
"github.com/pulumi/pulumi/sdk/v3/go/common/util/contract"
)

// Initialize a representation of the upstream provider.
//
// Lightly modifies the upstream provider to fix behavior that did not yet make it upstream.
//
// NOTE that this code runs in two contexts, during schema generation and during provider startup at runtime. The
// runtime startup is somewhat performance sensitive. Therefore any modifications undertaken here should complete
// quickly.
func newUpstreamProvider(ctx context.Context) awsShim.UpstreamProvider {
upstreamProvider, err := awsShim.NewUpstreamProvider(ctx)
contract.AssertNoErrorf(err, "NewUpstreamProvider failed to initialize")
rds.ReconfigureResources(upstreamProvider.SDKV2Provider)
return upstreamProvider
}
11 changes: 6 additions & 5 deletions sdk/dotnet/Rds/ParameterGroup.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 6 additions & 5 deletions sdk/go/aws/rds/parameterGroup.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 6 additions & 5 deletions sdk/java/src/main/java/com/pulumi/aws/rds/ParameterGroup.java

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit caf7ae5

Please sign in to comment.