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

Fix rds db param group default #3802

Merged
merged 10 commits into from
Apr 12, 2024
Merged
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
3 changes: 2 additions & 1 deletion provider/cmd/pulumi-resource-aws/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -120962,7 +120962,8 @@
"properties": {
"applyMethod": {
"type": "string",
"description": "\"immediate\" (default), or \"pending-reboot\". Some\nengines can't apply some parameters without a reboot, and you will need to\nspecify \"pending-reboot\" here.\n"
"description": "\"immediate\" (default), or \"pending-reboot\". Some\nengines can't apply some parameters without a reboot, and you will need to\nspecify \"pending-reboot\" here.\n",
"default": "immediate"
},
"name": {
"type": "string",
Expand Down
1 change: 1 addition & 0 deletions provider/pkg/rds/parameter_group.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ func parameterGroupReconfigure(p *schema.Provider) {
}
// 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
parameterGroup.Schema["parameter"].Elem.(*schema.Resource).Schema["apply_method"].Default = nil
// Need to exclude apply_method from the set hash.
oldSetFunc := parameterGroup.Schema["parameter"].Set
parameterGroup.Schema["parameter"].Set = parameterGroupParameterSetFunc(oldSetFunc)
Expand Down
90 changes: 69 additions & 21 deletions provider/provider_yaml_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,25 +141,9 @@ func TestRdsParameterGroupUnclearDiff(t *testing.T) {
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
}
type yamlProgram string

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 := `
const yaml yamlProgram = `
name: project
runtime: yaml
config:
Expand All @@ -184,13 +168,67 @@ resources:
value: "1"
`

const noApplyYaml yamlProgram = `
name: project
runtime: yaml
config:
value:
type: string
randSuffix:
type: string
resources:
default:
type: aws:rds/parameterGroup:ParameterGroup
properties:
name: securitygroup${randSuffix}
family: postgres14
parameters:
- name: track_io_timing
Copy link
Member

Choose a reason for hiding this comment

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

Nice this looks like it's testing the case where the default might kick in.

value: ${value}
- name: "log_checkpoints"
applyMethod: "pending-reboot"
value: "1"
`

type applyMethod string

var immediate applyMethod = "immediate"
var pendingReboot applyMethod = "pending-reboot"

type testCase struct {
name string
applyMethod1 *applyMethod
value1 string
file1 yamlProgram
applyMethod2 *applyMethod
value2 string
file2 yamlProgram
expectChanges bool
}

testCases := []testCase{
{"non-nil apply method, apply method change", &immediate, "1", yaml, &pendingReboot, "1", yaml, false},
{"non-nil apply method, value change", &immediate, "1", yaml, &immediate, "0", yaml, true},
{"non-nil apply method, both change", &immediate, "1", yaml, &pendingReboot, "0", yaml, true},
{"non-nil to nil apply method, apply method change", &pendingReboot, "1", yaml, nil, "1", noApplyYaml, false},
{"non-nil to nil apply method, value change", &immediate, "1", yaml, nil, "0", noApplyYaml, true},
{"non-nil to nil apply method, both change", &immediate, "1", yaml, nil, "0", noApplyYaml, true},
{"nil to non-nil apply method, apply method change", nil, "1", noApplyYaml, &pendingReboot, "1", yaml, false},
{"nil to non-nil apply method, value change", nil, "1", noApplyYaml, &immediate, "0", yaml, true},
{"nil to non-nil apply method, both change", nil, "1", noApplyYaml, &immediate, "0", yaml, true},
{"nil apply method, value change", nil, "1", noApplyYaml, nil, "0", noApplyYaml, true},
}

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

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)
err := os.WriteFile(filepath.Join(workdir, "Pulumi.yaml"), []byte(tc.file1), 0o600)
require.NoError(t, err)

pt := pulumitest.NewPulumiTest(t, workdir,
Expand All @@ -201,14 +239,24 @@ resources:

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

pt.SetConfig("applyMethod", tc.applyMethod1)
if tc.applyMethod1 != nil {
pt.SetConfig("applyMethod", string(*tc.applyMethod1))
}
pt.SetConfig("value", tc.value1)

pt.Up()

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

pt.SetConfig("applyMethod", tc.applyMethod2)
err = os.WriteFile(filepath.Join(workdir, "Pulumi.yaml"), []byte(tc.file2), 0o600)
require.NoError(t, err)

if tc.applyMethod2 != nil {
if tc.file2 == noApplyYaml {
t.Errorf("WRONG FILE!")
}
pt.SetConfig("applyMethod", string(*tc.applyMethod2))
}
pt.SetConfig("value", tc.value2)

if tc.expectChanges {
Expand Down
12 changes: 12 additions & 0 deletions provider/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -3093,6 +3093,18 @@ func ProviderFromMeta(metaInfo *tfbridge.MetadataInfo) *tfbridge.ProviderInfo {
"description": {
Default: managedByPulumi,
},
"parameter" : {
Elem: &tfbridge.SchemaInfo{
Fields: map[string]*tfbridge.SchemaInfo{
"apply_method": {
// We set the default value in the overlay since
// we remove it in the TF schema in
// provider/pkg/rds/parameter_group.go
Default: &tfbridge.DefaultInfo{Value: "immediate"},
Copy link
Member

Choose a reason for hiding this comment

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

Looks like this compensates for the patch. Very nice.

},
},
},
},
},
Docs: rds.ParameterGroupDocs("upstream"),
},
Expand Down
1 change: 1 addition & 0 deletions sdk/dotnet/Rds/Inputs/ParameterGroupParameterArgs.cs

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

1 change: 1 addition & 0 deletions sdk/dotnet/Rds/Inputs/ParameterGroupParameterGetArgs.cs

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

24 changes: 24 additions & 0 deletions sdk/go/aws/rds/pulumiTypes.go

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

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

11 changes: 11 additions & 0 deletions sdk/nodejs/types/input.ts

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

11 changes: 11 additions & 0 deletions sdk/nodejs/types/output.ts

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

2 changes: 2 additions & 0 deletions sdk/python/pulumi_aws/rds/_inputs.py

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

2 changes: 2 additions & 0 deletions sdk/python/pulumi_aws/rds/outputs.py

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

Loading