[RDS] BackupRetentionPeriod cannot be set to a blank value

[morremeyer]

Describe the bug

We use the aws terraform provider to manage RDS instances and clusters.
We also use AWS Backup for backups.

When using RDS with AWS Backup, the BackupRetentionPeriod needs to be set to either the exact value used by AWS Backup (which leads the idea of configuring it exactly once ad absurdum) or an empty value.

The aws terraform provider uses the AWS go SDK, which does not allow blank values since it defines the BackupRetentionPeriod as int64 (see https://github.com/aws/aws-sdk-go/blob/v1.45.23/service/rds/api.go#L41333).

However, the API clearly defines the value as optional, so it should be possible to not define it

Expected Behavior

A way to not set the BackupRetentionPeriod

Current Behavior

BackupRetentionPeriod is always set

Reproduction Steps

Can be seen in hashicorp/terraform-provider-aws#33465.

Possible Solution

Make BackupRetentionPeriod a pointer to an int64 value and allow it to be nil

Additional Information/Context

This surfaced initially in hashicorp/terraform-provider-aws#33465

SDK version used

v1.45.19

Environment details (Version of Go (go version)? OS name and version, etc.)

1.20, independent of OS

[rittneje]

@morremeyer I'm confused. ModifyDBClusterInput.BackupRetentionPeriod, which I believe is the field you are referring to, is already defined as *int64 not int64.

[morremeyer]

@rittneje You're right, it's already a pointer. However, according to the documentation for CreateDBClusterInput, it cannot be blank:

	// The number of days for which automated backups are retained.
	//
	// Valid for Cluster Type: Aurora DB clusters and Multi-AZ DB clusters
	//
	// Default: 1
	//
	// Constraints:
	//
	//    * Must be a value from 1 to 35.
	BackupRetentionPeriod *int64 `type:"integer"`

The same applies to ModifyDBClusterInput:

	// The number of days for which automated backups are retained. Specify a minimum
	// value of 1.
	//
	// Valid for Cluster Type: Aurora DB clusters and Multi-AZ DB clusters
	//
	// Default: 1
	//
	// Constraints:
	//
	//    * Must be a value from 1 to 35.
	BackupRetentionPeriod *int64 `type:"integer"`

Does this only apply when setting the value and it can also be left as nil pointer?

[RanVaknin]

Hi @morremeyer ,

Thanks for opening the issue.

according to the documentation for CreateDBClusterInput, it cannot be blank

You are missing the bit at the bottom which says that its optional. What this type definition means is that, BackupRetentionPeriod is in fact not required, but if it is set the value has to be between 1-35 (or in the SDKs case, a pointer to an int64 of value between 1-35) ,and it also it can be nil.

I'm not sure what the issue in terraform is, but using the Go SDK you can send a ModifyDBCluster request without getting a validation error using a nil value or no value at all.

package main

import (
	"github.com/aws/aws-sdk-go/aws"
	"github.com/aws/aws-sdk-go/aws/session"
	"github.com/aws/aws-sdk-go/service/rds"
)

func main() {
	sess, err := session.NewSession(&aws.Config{
		Region:   aws.String("us-east-1"),
		LogLevel: aws.LogLevel(aws.LogDebugWithHTTPBody),
	})
	if err != nil {
		panic(err)
	}

	client := rds.New(sess)

	_, err = client.ModifyDBCluster(&rds.ModifyDBClusterInput{
		DBClusterIdentifier: aws.String("foo"),
	})
	if err != nil {
		panic(err)
	}

}
/*
---[ REQUEST POST-SIGN ]-----------------------------
POST / HTTP/1.1
Host: rds.us-east-1.amazonaws.com
User-Agent: aws-sdk-go/1.45.20 (go1.19.1; darwin; arm64)
Content-Length: 65
Authorization: AWS4-HMAC-SHA256 Credential=REDACTED/20231010/us-east-1/rds/aws4_request, SignedHeaders=content-length;content-type;host;x-amz-date, Signature=REDACTED
Content-Type: application/x-www-form-urlencoded; charset=utf-8
X-Amz-Date: 20231010T202306Z
Accept-Encoding: gzip

Action=ModifyDBCluster&DBClusterIdentifier=foo&Version=2014-10-31
*/

Same for setting the BackupRetentionPeriod to nil, it will result in the value not getting serialized:

package main

import (
	"github.com/aws/aws-sdk-go/aws"
	"github.com/aws/aws-sdk-go/aws/session"
	"github.com/aws/aws-sdk-go/service/rds"
)

func main() {
	sess, err := session.NewSession(&aws.Config{
		Region:   aws.String("us-east-1"),
		LogLevel: aws.LogLevel(aws.LogDebugWithHTTPBody),
	})
	if err != nil {
		panic(err)
	}

	client := rds.New(sess)

	_, err = client.ModifyDBCluster(&rds.ModifyDBClusterInput{
		BackupRetentionPeriod: nil,
		DBClusterIdentifier:   aws.String("foo"),
	})
	if err != nil {
		panic(err)
	}

}
/*
---[ REQUEST POST-SIGN ]-----------------------------
POST / HTTP/1.1
Host: rds.us-east-1.amazonaws.com
User-Agent: aws-sdk-go/1.45.20 (go1.19.1; darwin; arm64)
Content-Length: 65
Authorization: AWS4-HMAC-SHA256 Credential=REDACTED/20231010/us-east-1/rds/aws4_request, SignedHeaders=content-length;content-type;host;x-amz-date, Signature=REDACTED
Content-Type: application/x-www-form-urlencoded; charset=utf-8
X-Amz-Date: 20231010T202456Z
Accept-Encoding: gzip

Action=ModifyDBCluster&DBClusterIdentifier=foo&Version=2014-10-31
*/

Values that are required, like DBClusterIdentifier cannot be nil or omitted, and will result in a client side validation error:

package main

import (
	"github.com/aws/aws-sdk-go/aws"
	"github.com/aws/aws-sdk-go/aws/session"
	"github.com/aws/aws-sdk-go/service/rds"
)

func main() {
	sess, err := session.NewSession(&aws.Config{
		Region:   aws.String("us-east-1"),
		LogLevel: aws.LogLevel(aws.LogDebugWithHTTPBody),
	})
	if err != nil {
		panic(err)
	}

	client := rds.New(sess)

	_, err = client.ModifyDBCluster(&rds.ModifyDBClusterInput{
		DBClusterIdentifier: nil,
	})
	if err != nil {
		panic(err)
	}
}
/*
panic: InvalidParameter: 1 validation error(s) found.
- missing required field, ModifyDBClusterInput.DBClusterIdentifier.
*/

Are you seeing any server side errors if you are passing nil?

Thanks,
Ran~

[morremeyer]

@RanVaknin Thank you for the detailed response and pointing out the code. It looks like it's purely on the terraform provider side then and I didn't check well enough on the SDK.

Sorry for the fuzz! I'm going to close this issue, if we're still running into errors after the terraform provider side issue is resolved, I'm going to reopen.

[github-actions]

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.