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

add name and description to SSM Maintenance Window Tasks #5762

Merged
merged 7 commits into from
Nov 14, 2018
Merged
Show file tree
Hide file tree
Changes from 4 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
24 changes: 24 additions & 0 deletions aws/resource_aws_ssm_maintenance_window_task.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,20 @@ func resourceAwsSsmMaintenanceWindowTask() *schema.Resource {
},
},

"name": {
Type: schema.TypeString,
Optional: true,
ForceNew: true,
Copy link
Contributor

@Ninir Ninir Sep 26, 2018

Choose a reason for hiding this comment

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

Is this really forcing a new resource? The API seems to tell the opposite (i.e. it's updatable): https://docs.aws.amazon.com/systems-manager/latest/APIReference/API_UpdateMaintenanceWindowTask.html

ValidateFunc: validateAwsSSMMaintenanceWindowTaskName,
},
Guimove marked this conversation as resolved.
Show resolved Hide resolved

"description": {
Type: schema.TypeString,
Optional: true,
ForceNew: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

ValidateFunc: validateAwsSSMMaintenanceWindowTaskDescription,
Copy link
Contributor

Choose a reason for hiding this comment

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

We can use an upstream validation function here instead of our own custom one: https://godoc.org/github.com/hashicorp/terraform/helper/validation#StringLenBetween

ValidateFunc: validation.StringLenBetween(0, 128),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok for the description but for the name I can keep my validator that check the regex and the length ?

},
Guimove marked this conversation as resolved.
Show resolved Hide resolved

"priority": {
Type: schema.TypeInt,
Optional: true,
Expand Down Expand Up @@ -191,6 +205,8 @@ func resourceAwsSsmMaintenanceWindowTaskCreate(d *schema.ResourceData, meta inte
TaskType: aws.String(d.Get("task_type").(string)),
ServiceRoleArn: aws.String(d.Get("service_role_arn").(string)),
TaskArn: aws.String(d.Get("task_arn").(string)),
Name: aws.String(d.Get("name").(string)),
Description: aws.String(d.Get("description").(string)),
Targets: expandAwsSsmTargets(d.Get("targets").([]interface{})),
}

Expand Down Expand Up @@ -241,6 +257,14 @@ func resourceAwsSsmMaintenanceWindowTaskRead(d *schema.ResourceData, meta interf
d.Set("task_arn", t.TaskArn)
d.Set("priority", t.Priority)

if t.Name != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

d.Set() automatically handles conversions of types like *string and we prefer to always call d.Set() for drift detection. (Same with description below)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok I'll remove that if

d.Set("name", t.Name)
}

if t.Description != nil {
d.Set("description", t.Description)
}

if t.LoggingInfo != nil {
if err := d.Set("logging_info", flattenAwsSsmMaintenanceWindowLoggingInfo(t.LoggingInfo)); err != nil {
return fmt.Errorf("[DEBUG] Error setting logging_info error: %#v", err)
Expand Down
6 changes: 6 additions & 0 deletions aws/resource_aws_ssm_maintenance_window_task_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ func TestAccAWSSSMMaintenanceWindowTask_basic(t *testing.T) {
Config: testAccAWSSSMMaintenanceWindowTaskBasicConfig(name),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSSSMMaintenanceWindowTaskExists("aws_ssm_maintenance_window_task.target", &task),
resource.TestCheckResourceAttr("aws_ssm_maintenance_window_task.target", "name", "TestMaintenanceWindowTask"),
resource.TestCheckResourceAttr("aws_ssm_maintenance_window_task.target", "description", "This ressource is for test purpose only"),
),
},
},
Expand Down Expand Up @@ -141,6 +143,8 @@ resource "aws_ssm_maintenance_window_task" "target" {
task_type = "RUN_COMMAND"
task_arn = "AWS-RunShellScript"
priority = 1
name = "TestMaintenanceWindowTask"
Guimove marked this conversation as resolved.
Show resolved Hide resolved
description = "This ressource is for test purpose only"
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: Typo ressource should be resource

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the french touch hehe

service_role_arn = "${aws_iam_role.ssm_role.arn}"
max_concurrency = "2"
max_errors = "1"
Expand Down Expand Up @@ -213,6 +217,8 @@ resource "aws_ssm_maintenance_window_task" "target" {
task_type = "RUN_COMMAND"
task_arn = "AWS-RunShellScript"
priority = 1
name = "TestMaintenanceWindowTask"
description = "This ressource is for test purpose only"
service_role_arn = "${aws_iam_role.ssm_role.arn}"
max_concurrency = "2"
max_errors = "1"
Expand Down
23 changes: 23 additions & 0 deletions aws/validators.go
Original file line number Diff line number Diff line change
Expand Up @@ -1568,6 +1568,29 @@ func validateAwsSSMName(v interface{}, k string) (ws []string, errors []error) {
return
}

func validateAwsSSMMaintenanceWindowTaskName(v interface{}, k string) (ws []string, errors []error) {
// https://docs.aws.amazon.com/systems-manager/latest/APIReference/API_RegisterTaskWithMaintenanceWindow.html#systemsmanager-RegisterTaskWithMaintenanceWindow-request-Name
value := v.(string)

if !regexp.MustCompile(`^[a-zA-Z0-9_\-.]{3,128}$`).MatchString(value) {
errors = append(errors, fmt.Errorf(
"Only alphanumeric characters, hyphens, dots & underscores allowed in %q: %q (Must satisfy regular expression pattern: ^[a-zA-Z0-9_\\-.]{3,128}$)",
k, value))
}

return
}

func validateAwsSSMMaintenanceWindowTaskDescription(v interface{}, k string) (ws []string, errors []error) {
// https://docs.aws.amazon.com/systems-manager/latest/APIReference/API_RegisterTaskWithMaintenanceWindow.html#systemsmanager-RegisterTaskWithMaintenanceWindow-request-Description
value := v.(string)
if len(value) > 128 {
errors = append(errors, fmt.Errorf(
"%q cannot be longer than 128 characters: %q", k, value))
}
return
}

func validateBatchName(v interface{}, k string) (ws []string, errors []error) {
value := v.(string)
if !regexp.MustCompile(`^[0-9a-zA-Z]{1}[0-9a-zA-Z_\-]{0,127}$`).MatchString(value) {
Expand Down