r/aws_ssm_maintenance_window_target: Change MaxItems of targets

[atsushi-ishibashi]

#2225
Not yet testacc.
According to Docs, minimum number is 0.
But I got a validation error and I'm asking aws-sdk-go.

ValidationException: 1 validation error detected: Value '[]' at 'targets' failed to satisfy constraint: Member must have length greater than or equal to 1

[Ninir]

Hi @atsushi-ishibashi

Thanks a lot for the work :)
Even though it is WIP, just left comments as it might help you!

[Ninir]

As you said the documentation exposes that this seems required.

Let's keep it required here, the documentation is wrong on the number of items being 0. (you can even provide this feedback to AWS if you want :) )

[atsushi-ishibashi]

Sure👍

[Ninir]

Nitpick: To be more memory-efficient, we could set: make(map[string]interface{}, 1), thoughts? :)

[atsushi-ishibashi]

It seems the same

func hoge1() {
	results := make([]map[string]interface{}, 100)
	for i := range results {
		t := make(map[string]interface{})
		t["key"], t["values"] = "key", "value"
		results[i] = t
	}
}

func hoge2() {
	results := make([]map[string]interface{}, 100)
	for i := range results {
		t := make(map[string]interface{}, 1)
		t["key"], t["values"] = "key", "value"
		results[i] = t
	}
}

func hoge3() {
	results := make([]map[string]interface{}, 100)
	for i := range results {
		t := map[string]interface{}{}
		t["key"], t["values"] = "key", "value"
		results[i] = t
	}
}

func BenchmarkHoge1(b *testing.B) {
	for i := 0; i < b.N; i++ {
		hoge1()
	}
}

func BenchmarkHoge2(b *testing.B) {
	for i := 0; i < b.N; i++ {
		hoge2()
	}
}

func BenchmarkHoge3(b *testing.B) {
	for i := 0; i < b.N; i++ {
		hoge3()
	}
}

go test -bench BenchmarkHoge -benchmem
BenchmarkHoge1-4   	   50000	     32459 ns/op	   33600 B/op	     200 allocs/op
BenchmarkHoge2-4   	   50000	     32529 ns/op	   33600 B/op	     200 allocs/op
BenchmarkHoge3-4   	   50000	     36636 ns/op	   33600 B/op	     200 allocs/op

[Ninir]

Third option is even better, interesting 🤔
Still, can you go for the second one, since it has a fixed size?

[atsushi-ishibashi]

Ok👍

[atsushi-ishibashi]

make testacc TEST=./aws TESTARGS='-run=TestAccAWSSSMMaintenanceWindowTarget_basic'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSSSMMaintenanceWindowTarget_basic -timeout 120m
=== RUN   TestAccAWSSSMMaintenanceWindowTarget_basic
--- PASS: TestAccAWSSSMMaintenanceWindowTarget_basic (52.91s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	52.970s

[Ninir]

Can you remove the first part of the change, since it is required? thanks

[Ninir]

Hi @atsushi-ishibashi

Just added 2 checks in an acceptance test to ensure we do have multiple target in the state.

make testacc TEST=./aws TESTARGS='-run=TestAccAWSSSMMaintenanceWindow'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSSSMMaintenanceWindow -timeout 120m
=== RUN   TestAccAWSSSMMaintenanceWindowTarget_basic
--- PASS: TestAccAWSSSMMaintenanceWindowTarget_basic (26.65s)
=== RUN   TestAccAWSSSMMaintenanceWindowTask_basic
--- PASS: TestAccAWSSSMMaintenanceWindowTask_basic (123.93s)
=== RUN   TestAccAWSSSMMaintenanceWindowTask_updateForcesNewResource
--- PASS: TestAccAWSSSMMaintenanceWindowTask_updateForcesNewResource (151.63s)
=== RUN   TestAccAWSSSMMaintenanceWindow_basic
--- PASS: TestAccAWSSSMMaintenanceWindow_basic (38.68s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	340.963s

Looks very good to me, thanks for the work & reactivity! 👍

[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!