resource/aws_db_option_group: Add version field

[chrisminton]

Fixes #748

Certain Oracle RDS options require a version attribute set, e.g. OEM_AGENT. This change adds that attribute to the option.

The acceptance test is wrong since I'm not sure about how to get the correct option number (obviously not option.0.version) from state. If anyone can give me a steer it would be appreciated.

[Ninir]

Hi @chrisminton

This is looking good!

Just left a few comments to address / discuss before approving & merging.

[Ninir]

Can this attribute be retrieved by the AWS API without a user setting it?
If so, this should also have a Computed: true, option

[Ninir]

As option is of type TypeSet (https://github.com/terraform-providers/terraform-provider-aws/pull/2590/files#diff-e61872fad819968684cae97c19506df5R70), a given entry can be retrieved with a hash, as in: https://github.com/chrisminton/terraform-provider-aws/blob/89c8a140afce2bc728b7e3e07c526f961c02f875/aws/resource_aws_db_option_group_test.go#L226

What I usually do is get the hash from the logs, using TF_LOG=DEBUG with TF_LOG_PATH=terraform.log as in:

TF_LOG=DEBUG make testacc TEST=./aws TESTARGS='-run= TestAccAWSDBOptionGroup_multipleOptions'

This will put multiple log lines in the aws/terraform.log file, making it easier for you to debug :)

Note that the hash will change if parts of the option change.

[Ninir]

Hi @chrisminton

Do you think you would have time to fix the comments so that we get this merged?

Thanks!

[chrisminton]

Hi @Ninir - yes! I've been caught up in other matters. It looks to me that the hash changes every time in the debug logs so this may not be a useful test but i'll test again. The version attribute does not require Computed: true, it must be user-set.

[nmische]

Is there any update on this issue? This is blocking us from being able to manage Oracle RDS with Terraform. Thanks.

[bflad]

Thanks for your work here @chrisminton! I have a commit following yours which fixes the testing. Since we don't know the hash value, we can check the returned *rds.OptionGroup directly:

func testAccCheckAWSDBOptionGroupOptionVersionAttribute(optionGroup *rds.OptionGroup, optionVersion string) resource.TestCheckFunc {
	return func(s *terraform.State) error {
		if optionGroup == nil {
			return errors.New("Option Group does not exist")
		}
		if len(optionGroup.Options) == 0 {
			return errors.New("Option Group does not have any options")
		}
		foundOptionVersion := aws.StringValue(optionGroup.Options[0].OptionVersion)
		if foundOptionVersion != optionVersion {
			return fmt.Errorf("Expected option version %q and received %q", optionVersion, foundOptionVersion)
		}
		return nil
	}
}

It also needed to only optionally be added to the TypeSet hash (otherwise would show as a difference when anyone upgraded):

if v, ok := m["version"]; ok && v.(string) != "" {
	buf.WriteString(fmt.Sprintf("%s-", v.(string)))
}

With that, we're all set!

make testacc TEST=./aws TESTARGS='-run=TestAccAWSDBOptionGroup'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSDBOptionGroup -timeout 120m
=== RUN   TestAccAWSDBOptionGroup_importBasic
--- PASS: TestAccAWSDBOptionGroup_importBasic (14.62s)
=== RUN   TestAccAWSDBOptionGroup_basic
--- PASS: TestAccAWSDBOptionGroup_basic (12.23s)
=== RUN   TestAccAWSDBOptionGroup_timeoutBlock
--- PASS: TestAccAWSDBOptionGroup_timeoutBlock (14.70s)
=== RUN   TestAccAWSDBOptionGroup_namePrefix
--- PASS: TestAccAWSDBOptionGroup_namePrefix (13.23s)
=== RUN   TestAccAWSDBOptionGroup_generatedName
--- PASS: TestAccAWSDBOptionGroup_generatedName (12.33s)
=== RUN   TestAccAWSDBOptionGroup_defaultDescription
--- PASS: TestAccAWSDBOptionGroup_defaultDescription (12.45s)
=== RUN   TestAccAWSDBOptionGroup_basicDestroyWithInstance
--- PASS: TestAccAWSDBOptionGroup_basicDestroyWithInstance (482.69s)
=== RUN   TestAccAWSDBOptionGroup_OptionSettings
--- PASS: TestAccAWSDBOptionGroup_OptionSettings (11.07s)
=== RUN   TestAccAWSDBOptionGroup_sqlServerOptionsUpdate
--- PASS: TestAccAWSDBOptionGroup_sqlServerOptionsUpdate (23.07s)
=== RUN   TestAccAWSDBOptionGroup_OracleOptionsUpdate
--- PASS: TestAccAWSDBOptionGroup_OracleOptionsUpdate (31.28s)
=== RUN   TestAccAWSDBOptionGroup_multipleOptions
--- PASS: TestAccAWSDBOptionGroup_multipleOptions (13.87s)

[chrisminton]

Thanks @bflad and sorry I didn't have any time to get back to this!

[nmische]

Thanks for getting this merged!

[bflad]

This has been released in version 1.15.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!