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

TimestreamInfluxDB DB instance add more updatable arguments #7

91 changes: 76 additions & 15 deletions internal/service/timestreaminfluxdb/db_instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package timestreaminfluxdb
import (
"context"
"errors"
"math"
"time"

"github.com/YakDriver/regexache"
Expand Down Expand Up @@ -105,11 +106,8 @@ func (r *resourceDBInstance) Schema(ctx context.Context, req resource.SchemaRequ
that each data point persists). A bucket belongs to an organization.`,
},
"db_instance_type": schema.StringAttribute{
CustomType: fwtypes.StringEnumType[awstypes.DbInstanceType](),
Required: true,
PlanModifiers: []planmodifier.String{
stringplanmodifier.RequiresReplace(),
},
CustomType: fwtypes.StringEnumType[awstypes.DbInstanceType](),
Required: true,
Description: `The Timestream for InfluxDB DB instance type to run InfluxDB on.`,
},
"db_parameter_group_identifier": schema.StringAttribute{
Expand Down Expand Up @@ -146,7 +144,6 @@ func (r *resourceDBInstance) Schema(ctx context.Context, req resource.SchemaRequ
Optional: true,
Computed: true,
PlanModifiers: []planmodifier.String{
stringplanmodifier.RequiresReplace(),
stringplanmodifier.UseStateForUnknown(),
},
Description: `Specifies whether the DB instance will be deployed as a standalone instance or
Expand Down Expand Up @@ -187,6 +184,18 @@ func (r *resourceDBInstance) Schema(ctx context.Context, req resource.SchemaRequ
prefix included in the endpoint. DB instance names must be unique per customer
and per region.`,
},
"network_type": schema.StringAttribute{
CustomType: fwtypes.StringEnumType[awstypes.NetworkType](),
Optional: true,
Computed: true,
PlanModifiers: []planmodifier.String{
stringplanmodifier.RequiresReplace(),
stringplanmodifier.UseStateForUnknown(),
},
Description: `Specifies whether the networkType of the Timestream for InfluxDB instance is
IPV4, which can communicate over IPv4 protocol only, or DUAL, which can communicate
over both IPv4 and IPv6 protocols.`,
},
names.AttrTags: tftags.TagsAttribute(),
names.AttrTagsAll: tftags.TagsAttributeComputedOnly(),
"organization": schema.StringAttribute{
Expand Down Expand Up @@ -215,6 +224,18 @@ func (r *resourceDBInstance) Schema(ctx context.Context, req resource.SchemaRequ
also use the InfluxDB CLI to create an operator token. These attributes will be
stored in a Secret created in AWS SecretManager in your account.`,
},
names.AttrPort: schema.Int64Attribute{
Optional: true,
Computed: true,

Choose a reason for hiding this comment

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

would it be redundant to add default here as well?

Suggested change
Computed: true,
Computed: true,
Default: int64(8086),

Copy link
Author

Choose a reason for hiding this comment

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

It would be redundant, the port defaults to 8086 anyways when creating an instance.

PlanModifiers: []planmodifier.Int64{
int64planmodifier.UseStateForUnknown(),
},
Validators: []validator.Int64{
int64validator.Between(1024, 65535),
int64validator.NoneOf(2375, 2376, 7788, 7789, 7790, 7791, 7792, 7793, 7794, 7795, 7796, 7797, 7798, 7799, 8090, 51678, 51679, 51680),

Choose a reason for hiding this comment

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

(jk)

Suggested change
int64validator.NoneOf(2375, 2376, 7788, 7789, 7790, 7791, 7792, 7793, 7794, 7795, 7796, 7797, 7798, 7799, 8090, 51678, 51679, 51680),
int64validator.NoneOf(1337, 2375, 2376, 7788, 7789, 7790, 7791, 7792, 7793, 7794, 7795, 7796, 7797, 7798, 7799, 8090, 51678, 51679, 51680),

Copy link
Author

Choose a reason for hiding this comment

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

Is that a leetspeak reference?

},
Description: `The port number on which InfluxDB accepts connections.`,
},
names.AttrPubliclyAccessible: schema.BoolAttribute{
Optional: true,
Computed: true,
Expand All @@ -226,9 +247,7 @@ func (r *resourceDBInstance) Schema(ctx context.Context, req resource.SchemaRequ
},
"secondary_availability_zone": schema.StringAttribute{
Computed: true,
PlanModifiers: []planmodifier.String{
stringplanmodifier.UseStateForUnknown(),
},
Default: nil,
Description: `The Availability Zone in which the standby instance is located when deploying
with a MultiAZ standby instance.`,
},
Expand Down Expand Up @@ -454,15 +473,49 @@ func (r *resourceDBInstance) Update(ctx context.Context, req resource.UpdateRequ
}

if !plan.DBParameterGroupIdentifier.Equal(state.DBParameterGroupIdentifier) ||
!plan.LogDeliveryConfiguration.Equal(state.LogDeliveryConfiguration) {
!plan.LogDeliveryConfiguration.Equal(state.LogDeliveryConfiguration) ||
!plan.DBInstanceType.Equal(state.DBInstanceType) ||
!plan.DeploymentType.Equal(state.DeploymentType) ||
!plan.Port.Equal(state.Port) {
in := timestreaminfluxdb.UpdateDbInstanceInput{
Identifier: plan.ID.ValueStringPointer(),
}

resp.Diagnostics.Append(flex.Expand(ctx, plan, &in)...)
// If any argument is updated with the same value, a ValidationException will occur. Arguments should only
// be updated if they have changed. For this reason, flex.Expand cannot be used for all arguments.
if !plan.DBParameterGroupIdentifier.Equal(state.DBParameterGroupIdentifier) {
in.DbParameterGroupIdentifier = plan.DBParameterGroupIdentifier.ValueStringPointer()
}

if resp.Diagnostics.HasError() {
return
if !plan.LogDeliveryConfiguration.Equal(state.LogDeliveryConfiguration) {
flex.Expand(ctx, plan.LogDeliveryConfiguration, &in.LogDeliveryConfiguration)
}

if !plan.DBInstanceType.Equal(state.DBInstanceType) {
in.DbInstanceType = awstypes.DbInstanceType(plan.DBInstanceType.ValueString())
}

if !plan.DeploymentType.Equal(state.DeploymentType) {
in.DeploymentType = awstypes.DeploymentType(plan.DeploymentType.ValueString())
}

if !plan.Port.Equal(state.Port) {
if plan.Port.ValueInt64() > math.MaxInt32 {
err := errors.New("port was greater than the maximum allowed value for int32")
resp.Diagnostics.AddError(
create.ProblemStandardMessage(names.TimestreamInfluxDB, create.ErrActionUpdating, ResNameDBInstance, plan.ID.String(), err),
err.Error(),
)
return
} else if plan.Port.ValueInt64() < math.MinInt32 {
err := errors.New("port was less than the minimum allowed value for int32")
resp.Diagnostics.AddError(
create.ProblemStandardMessage(names.TimestreamInfluxDB, create.ErrActionUpdating, ResNameDBInstance, plan.ID.String(), err),
err.Error(),
)
return
}
in.Port = aws.Int32(int32(plan.Port.ValueInt64()))
forestmvey marked this conversation as resolved.
Show resolved Hide resolved
}

_, err := conn.UpdateDbInstance(ctx, &in)
Expand Down Expand Up @@ -494,6 +547,12 @@ func (r *resourceDBInstance) Update(ctx context.Context, req resource.UpdateRequ
plan.SecondaryAvailabilityZone = flex.StringToFrameworkLegacy(ctx, output.SecondaryAvailabilityZone)
}

// Updating tags can leave SecondaryAvailabilityZone unknown, as tags cannot be included in UpdateDbInstanceInput above.
// To get around this, if SecondaryAvailabilityZone is unknown after an update, set it to its previous value.
if plan.SecondaryAvailabilityZone.IsUnknown() {
plan.SecondaryAvailabilityZone = state.SecondaryAvailabilityZone
Copy link
Author

@trevorbonas trevorbonas Dec 17, 2024

Choose a reason for hiding this comment

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

This is the fix for secondary_availability_zone being left as unknown when tags are updated. If the new value (in plan) is unknown, set it to its old value (in state).

From what I can determine, the problem is that Update is called when tags are updated (for some reason) but UpdateDbInstance isn't actually called, as UpdateDbInstance doesn't allow tags to be updated, therefore secondary_availability_zone is left as unknown. I would like a better fix that scaled better for possible future computed values, but at this point I'm not sure what that better fix would be.

Choose a reason for hiding this comment

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

might be good to add a small comment regarding this in the code 👍

Copy link
Author

Choose a reason for hiding this comment

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

Added.

}

resp.Diagnostics.Append(resp.State.Set(ctx, &plan)...)
}

Expand Down Expand Up @@ -557,7 +616,7 @@ func waitDBInstanceCreated(ctx context.Context, conn *timestreaminfluxdb.Client,

func waitDBInstanceUpdated(ctx context.Context, conn *timestreaminfluxdb.Client, id string, timeout time.Duration) (*timestreaminfluxdb.GetDbInstanceOutput, error) {
stateConf := &retry.StateChangeConf{
Pending: enum.Slice(awstypes.StatusModifying, awstypes.StatusUpdating),
Pending: enum.Slice(awstypes.StatusModifying, awstypes.StatusUpdating, awstypes.StatusUpdatingInstanceType, awstypes.StatusUpdatingDeploymentType),
Target: enum.Slice(awstypes.StatusAvailable),
Refresh: statusDBInstance(ctx, conn, id),
Timeout: timeout,
Expand All @@ -575,7 +634,7 @@ func waitDBInstanceUpdated(ctx context.Context, conn *timestreaminfluxdb.Client,

func waitDBInstanceDeleted(ctx context.Context, conn *timestreaminfluxdb.Client, id string, timeout time.Duration) (*timestreaminfluxdb.GetDbInstanceOutput, error) {
stateConf := &retry.StateChangeConf{
Pending: enum.Slice(awstypes.StatusDeleting),
Pending: enum.Slice(awstypes.StatusDeleting, awstypes.StatusDeleted),
Target: []string{},
Refresh: statusDBInstance(ctx, conn, id),
Timeout: timeout,
Expand Down Expand Up @@ -643,8 +702,10 @@ type resourceDBInstanceData struct {
InfluxAuthParametersSecretARN types.String `tfsdk:"influx_auth_parameters_secret_arn"`
LogDeliveryConfiguration fwtypes.ListNestedObjectValueOf[logDeliveryConfigurationData] `tfsdk:"log_delivery_configuration"`
Name types.String `tfsdk:"name"`
NetworkType fwtypes.StringEnum[awstypes.NetworkType] `tfsdk:"network_type"`
Organization types.String `tfsdk:"organization"`
Password types.String `tfsdk:"password"`
Port types.Int64 `tfsdk:"port"`
PubliclyAccessible types.Bool `tfsdk:"publicly_accessible"`
SecondaryAvailabilityZone types.String `tfsdk:"secondary_availability_zone"`
Tags tftags.Map `tfsdk:"tags"`
Expand Down
107 changes: 100 additions & 7 deletions internal/service/timestreaminfluxdb/db_instance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ func TestAccTimestreamInfluxDBDBInstance_basic(t *testing.T) {
resource.TestCheckResourceAttr(resourceName, "db_storage_type", string(awstypes.DbStorageTypeInfluxIoIncludedT1)),
resource.TestCheckResourceAttr(resourceName, "deployment_type", string(awstypes.DeploymentTypeSingleAz)),
resource.TestCheckResourceAttrSet(resourceName, "influx_auth_parameters_secret_arn"),
resource.TestCheckResourceAttr(resourceName, "network_type", string(awstypes.NetworkTypeIpv4)),
resource.TestCheckResourceAttr(resourceName, "port", "8086"),
resource.TestCheckResourceAttr(resourceName, names.AttrPubliclyAccessible, acctest.CtFalse),
),
},
Expand Down Expand Up @@ -96,6 +98,57 @@ func TestAccTimestreamInfluxDBDBInstance_disappears(t *testing.T) {
})
}

func TestAccTimestreamInfluxDBDBInstance_dbInstanceType(t *testing.T) {
ctx := acctest.Context(t)
if testing.Short() {
t.Skip("skipping long-running test in short mode")
}

var dbInstance1, dbInstance2 timestreaminfluxdb.GetDbInstanceOutput
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
resourceName := "aws_timestreaminfluxdb_db_instance.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() {
acctest.PreCheck(ctx, t)
testAccPreCheck(ctx, t)
},
ErrorCheck: acctest.ErrorCheck(t, names.TimestreamInfluxDBServiceID),
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
CheckDestroy: testAccCheckDBInstanceDestroy(ctx),
Steps: []resource.TestStep{
{
Config: testAccDBInstanceConfig_dbInstanceType(rName, string(awstypes.DbInstanceTypeDbInfluxMedium)),
Check: resource.ComposeTestCheckFunc(
testAccCheckDBInstanceExists(ctx, resourceName, &dbInstance1),
resource.TestCheckResourceAttr(resourceName, "db_instance_type", string(awstypes.DbInstanceTypeDbInfluxMedium)),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{names.AttrBucket, names.AttrUsername, names.AttrPassword, "organization"},
},
{
Config: testAccDBInstanceConfig_dbInstanceType(rName, string(awstypes.DbInstanceTypeDbInfluxLarge)),
Check: resource.ComposeTestCheckFunc(
testAccCheckDBInstanceExists(ctx, resourceName, &dbInstance2),
testAccCheckDBInstanceNotRecreated(&dbInstance1, &dbInstance2),
testAccCheckDBInstanceExists(ctx, resourceName, &dbInstance2),
resource.TestCheckResourceAttr(resourceName, "db_instance_type", string(awstypes.DbInstanceTypeDbInfluxLarge)),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{names.AttrBucket, names.AttrUsername, names.AttrPassword, "organization"},
},
},
})
}

func TestAccTimestreamInfluxDBDBInstance_logDeliveryConfiguration(t *testing.T) {
ctx := acctest.Context(t)
if testing.Short() {
Expand Down Expand Up @@ -140,6 +193,12 @@ func TestAccTimestreamInfluxDBDBInstance_logDeliveryConfiguration(t *testing.T)
resource.TestCheckResourceAttr(resourceName, "log_delivery_configuration.0.s3_configuration.0.enabled", acctest.CtFalse),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{names.AttrBucket, names.AttrUsername, names.AttrPassword, "organization"},
},
},
})
}
Expand Down Expand Up @@ -181,13 +240,13 @@ func TestAccTimestreamInfluxDBDBInstance_publiclyAccessible(t *testing.T) {
})
}

func TestAccTimestreamInfluxDBDBInstance_deploymentTypeMultiAzStandby(t *testing.T) {
func TestAccTimestreamInfluxDBDBInstance_deploymentType(t *testing.T) {
ctx := acctest.Context(t)
if testing.Short() {
t.Skip("skipping long-running test in short mode")
}

var dbInstance timestreaminfluxdb.GetDbInstanceOutput
var dbInstance1, dbInstance2 timestreaminfluxdb.GetDbInstanceOutput
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
resourceName := "aws_timestreaminfluxdb_db_instance.test"

Expand All @@ -201,9 +260,9 @@ func TestAccTimestreamInfluxDBDBInstance_deploymentTypeMultiAzStandby(t *testing
CheckDestroy: testAccCheckDBInstanceDestroy(ctx),
Steps: []resource.TestStep{
{
Config: testAccDBInstanceConfig_deploymentTypeMultiAzStandby(rName),
Config: testAccDBInstanceConfig_deploymentType(rName, string(awstypes.DeploymentTypeWithMultiazStandby)),
Check: resource.ComposeTestCheckFunc(
testAccCheckDBInstanceExists(ctx, resourceName, &dbInstance),
testAccCheckDBInstanceExists(ctx, resourceName, &dbInstance1),
// DB instance will not be publicly accessible and will not have an endpoint.
// DB instance will have a secondary availability zone.
resource.TestCheckResourceAttrSet(resourceName, "secondary_availability_zone"),
Expand All @@ -216,6 +275,21 @@ func TestAccTimestreamInfluxDBDBInstance_deploymentTypeMultiAzStandby(t *testing
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{names.AttrBucket, names.AttrUsername, names.AttrPassword, "organization"},
},
{
Config: testAccDBInstanceConfig_deploymentType(rName, string(awstypes.DeploymentTypeSingleAz)),
Check: resource.ComposeTestCheckFunc(
testAccCheckDBInstanceExists(ctx, resourceName, &dbInstance2),
testAccCheckDBInstanceNotRecreated(&dbInstance1, &dbInstance2),
resource.TestCheckResourceAttr(resourceName, "secondary_availability_zone", ""),
resource.TestCheckResourceAttr(resourceName, "deployment_type", string(awstypes.DeploymentTypeSingleAz)),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{names.AttrBucket, names.AttrUsername, names.AttrPassword, "organization"},
},
},
})
}
Expand Down Expand Up @@ -313,12 +387,30 @@ resource "aws_timestreaminfluxdb_db_instance" "test" {
vpc_subnet_ids = aws_subnet.test[*].id
vpc_security_group_ids = [aws_security_group.test.id]
db_instance_type = "db.influx.medium"
port = 8086
bucket = "initial"
organization = "organization"
}
`, rName))
}

func testAccDBInstanceConfig_dbInstanceType(rName string, instanceType string) string {
return acctest.ConfigCompose(testAccDBInstanceConfig_base(rName, 1), fmt.Sprintf(`
resource "aws_timestreaminfluxdb_db_instance" "test" {
name = %[1]q
allocated_storage = 20
username = "admin"
password = "testpassword"
vpc_subnet_ids = aws_subnet.test[*].id
vpc_security_group_ids = [aws_security_group.test.id]
db_instance_type = %[2]q
port = 8086
bucket = "initial"
organization = "organization"
}
`, rName, instanceType))
}

// Configuration with log_delivery_configuration set and enabled.
func testAccDBInstanceConfig_logDeliveryConfigurationEnabled(rName string, enabled bool) string {
return acctest.ConfigCompose(testAccDBInstanceConfig_base(rName, 1), fmt.Sprintf(`
Expand Down Expand Up @@ -354,6 +446,7 @@ resource "aws_timestreaminfluxdb_db_instance" "test" {
vpc_security_group_ids = [aws_security_group.test.id]
db_instance_type = "db.influx.medium"
publicly_accessible = false
port = 8086
bucket = "initial"
organization = "organization"

Expand Down Expand Up @@ -407,7 +500,7 @@ resource "aws_timestreaminfluxdb_db_instance" "test" {
`, rName))
}

func testAccDBInstanceConfig_deploymentTypeMultiAzStandby(rName string) string {
func testAccDBInstanceConfig_deploymentType(rName string, deploymentType string) string {
return acctest.ConfigCompose(testAccDBInstanceConfig_base(rName, 2), fmt.Sprintf(`
resource "aws_timestreaminfluxdb_db_instance" "test" {
name = %[1]q
Expand All @@ -421,7 +514,7 @@ resource "aws_timestreaminfluxdb_db_instance" "test" {
bucket = "initial"
organization = "organization"

deployment_type = "WITH_MULTIAZ_STANDBY"
deployment_type = %[2]q
}
`, rName))
`, rName, deploymentType))
}
2 changes: 1 addition & 1 deletion internal/service/timestreaminfluxdb/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// SPDX-License-Identifier: MPL-2.0

//go:generate go run ../../generate/servicepackage/main.go
//go:generate go run ../../generate/tags/main.go -KVTValues -ListTags -ServiceTagsMap -UpdateTags
//go:generate go run ../../generate/tags/main.go -KVTValues -ListTags -ServiceTagsMap -UpdateTags -CreateTags
//go:generate go run ../../generate/tagstests/main.go
// ONLY generate directives and package declaration! Do not add anything else to this file.

Expand Down
9 changes: 9 additions & 0 deletions internal/service/timestreaminfluxdb/tags_gen.go

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

Loading
Loading