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 AWS AppConfig support #79

Merged
merged 25 commits into from
Mar 17, 2021
Merged

Add AWS AppConfig support #79

merged 25 commits into from
Mar 17, 2021

Conversation

Isaiah-Turner
Copy link

This PR adds AppConfig support to Terraform. See hashicorp#11973 for an overview of the configuration.

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccAWSAppConfig'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSAppConfig -timeout 120m
=== RUN   TestAccAWSAppConfigApplication_basic
=== PAUSE TestAccAWSAppConfigApplication_basic
=== RUN   TestAccAWSAppConfigApplication_disappears
=== PAUSE TestAccAWSAppConfigApplication_disappears
=== RUN   TestAccAWSAppConfigApplication_Tags
=== PAUSE TestAccAWSAppConfigApplication_Tags
=== RUN   TestAccAWSAppConfigConfigurationProfile_basic
=== PAUSE TestAccAWSAppConfigConfigurationProfile_basic
=== RUN   TestAccAWSAppConfigConfigurationProfile_disappears
=== PAUSE TestAccAWSAppConfigConfigurationProfile_disappears
=== RUN   TestAccAWSAppConfigConfigurationProfile_LocationURI_SSMParameter
=== PAUSE TestAccAWSAppConfigConfigurationProfile_LocationURI_SSMParameter
=== RUN   TestAccAWSAppConfigConfigurationProfile_LocationURI_SSMDocument
=== PAUSE TestAccAWSAppConfigConfigurationProfile_LocationURI_SSMDocument
=== RUN   TestAccAWSAppConfigConfigurationProfile_Validators
=== PAUSE TestAccAWSAppConfigConfigurationProfile_Validators
=== RUN   TestAccAWSAppConfigConfigurationProfile_Tags
=== PAUSE TestAccAWSAppConfigConfigurationProfile_Tags
=== RUN   TestAccAWSAppConfigDeploymentStrategy_basic
=== PAUSE TestAccAWSAppConfigDeploymentStrategy_basic
=== RUN   TestAccAWSAppConfigDeploymentStrategy_disappears
=== PAUSE TestAccAWSAppConfigDeploymentStrategy_disappears
=== RUN   TestAccAWSAppConfigDeploymentStrategy_Tags
=== PAUSE TestAccAWSAppConfigDeploymentStrategy_Tags
=== RUN   TestAccAWSAppConfigDeploymentStrategy_Growth
=== PAUSE TestAccAWSAppConfigDeploymentStrategy_Growth
=== RUN   TestAccAWSAppConfigDeploymentStrategy_ReplicateTo
=== PAUSE TestAccAWSAppConfigDeploymentStrategy_ReplicateTo
=== RUN   TestAccAWSAppConfigDeploymentStrategy_BakeTime
=== PAUSE TestAccAWSAppConfigDeploymentStrategy_BakeTime
=== RUN   TestAccAWSAppConfigDeployment_basic
=== PAUSE TestAccAWSAppConfigDeployment_basic
=== RUN   TestAccAWSAppConfigDeployment_disappears
=== PAUSE TestAccAWSAppConfigDeployment_disappears
=== RUN   TestAccAWSAppConfigDeployment_Tags
=== PAUSE TestAccAWSAppConfigDeployment_Tags
=== RUN   TestAccAWSAppConfigEnvironment_basic
=== PAUSE TestAccAWSAppConfigEnvironment_basic
=== RUN   TestAccAWSAppConfigEnvironment_disappears
=== PAUSE TestAccAWSAppConfigEnvironment_disappears
=== RUN   TestAccAWSAppConfigEnvironment_Monitors
=== PAUSE TestAccAWSAppConfigEnvironment_Monitors
=== RUN   TestAccAWSAppConfigEnvironment_Tags
=== PAUSE TestAccAWSAppConfigEnvironment_Tags
=== RUN   TestAccAWSAppConfigHostedConfigurationVersion_basic
--- PASS: TestAccAWSAppConfigHostedConfigurationVersion_basic (28.15s)
=== RUN   TestAccAWSAppConfigHostedConfigurationVersion_disappears
--- PASS: TestAccAWSAppConfigHostedConfigurationVersion_disappears (25.96s)
=== RUN   TestAccAWSAppConfigHostedConfigurationVersion_Plain
--- PASS: TestAccAWSAppConfigHostedConfigurationVersion_Plain (28.00s)
=== RUN   TestAccAWSAppConfigHostedConfigurationVersion_JSON
--- PASS: TestAccAWSAppConfigHostedConfigurationVersion_JSON (25.44s)
=== RUN   TestAccAWSAppConfigHostedConfigurationVersion_YAML
--- PASS: TestAccAWSAppConfigHostedConfigurationVersion_YAML (25.64s)
=== CONT  TestAccAWSAppConfigApplication_basic
=== CONT  TestAccAWSAppConfigDeploymentStrategy_Growth
=== CONT  TestAccAWSAppConfigEnvironment_basic
=== CONT  TestAccAWSAppConfigConfigurationProfile_LocationURI_SSMDocument
=== CONT  TestAccAWSAppConfigDeployment_basic
=== CONT  TestAccAWSAppConfigDeploymentStrategy_BakeTime
=== CONT  TestAccAWSAppConfigEnvironment_Tags
=== CONT  TestAccAWSAppConfigDeploymentStrategy_ReplicateTo
=== CONT  TestAccAWSAppConfigConfigurationProfile_basic
=== CONT  TestAccAWSAppConfigEnvironment_Monitors
=== CONT  TestAccAWSAppConfigConfigurationProfile_LocationURI_SSMParameter
=== CONT  TestAccAWSAppConfigDeployment_Tags
=== CONT  TestAccAWSAppConfigDeploymentStrategy_Tags
=== CONT  TestAccAWSAppConfigDeployment_disappears
=== CONT  TestAccAWSAppConfigConfigurationProfile_disappears
=== CONT  TestAccAWSAppConfigApplication_Tags
=== CONT  TestAccAWSAppConfigApplication_disappears
=== CONT  TestAccAWSAppConfigDeploymentStrategy_basic
=== CONT  TestAccAWSAppConfigConfigurationProfile_Tags
=== CONT  TestAccAWSAppConfigEnvironment_disappears
--- PASS: TestAccAWSAppConfigApplication_disappears (60.75s)
=== CONT  TestAccAWSAppConfigConfigurationProfile_Validators
--- PASS: TestAccAWSAppConfigEnvironment_basic (71.08s)
=== CONT  TestAccAWSAppConfigDeploymentStrategy_disappears
--- PASS: TestAccAWSAppConfigEnvironment_disappears (71.51s)
--- PASS: TestAccAWSAppConfigConfigurationProfile_disappears (71.58s)
--- PASS: TestAccAWSAppConfigConfigurationProfile_basic (73.05s)
--- PASS: TestAccAWSAppConfigEnvironment_Monitors (73.51s)
--- PASS: TestAccAWSAppConfigDeployment_disappears (76.24s)
--- PASS: TestAccAWSAppConfigDeployment_basic (76.49s)
--- PASS: TestAccAWSAppConfigApplication_basic (80.66s)
--- PASS: TestAccAWSAppConfigConfigurationProfile_LocationURI_SSMParameter (80.78s)
--- PASS: TestAccAWSAppConfigConfigurationProfile_LocationURI_SSMDocument (81.17s)
--- PASS: TestAccAWSAppConfigDeploymentStrategy_Growth (81.79s)
--- PASS: TestAccAWSAppConfigDeploymentStrategy_ReplicateTo (82.36s)
--- PASS: TestAccAWSAppConfigDeploymentStrategy_BakeTime (82.42s)
--- PASS: TestAccAWSAppConfigDeploymentStrategy_basic (82.64s)
--- PASS: TestAccAWSAppConfigEnvironment_Tags (102.24s)
--- PASS: TestAccAWSAppConfigDeploymentStrategy_disappears (31.67s)
--- PASS: TestAccAWSAppConfigDeployment_Tags (109.60s)
--- PASS: TestAccAWSAppConfigConfigurationProfile_Validators (58.56s)
--- PASS: TestAccAWSAppConfigDeploymentStrategy_Tags (126.00s)
--- PASS: TestAccAWSAppConfigConfigurationProfile_Tags (126.10s)
--- PASS: TestAccAWSAppConfigApplication_Tags (126.77s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	262.867s
...

@github-actions
Copy link

github-actions bot commented Mar 9, 2021

Thank you for your contribution! 🚀

Please note that typically Go dependency changes are handled in this repository by dependabot or the maintainers. This is to prevent pull request merge conflicts and further delay reviews of contributions. Remove any changes to the go.mod or go.sum files and commit them into this pull request.

Additional details:

  • Check open pull requests with the dependencies label to view other dependency updates.
  • If this pull request includes an update the AWS Go SDK (or any other dependency) version, only updates submitted via dependabot will be merged. This pull request will need to remove these changes and will need to be rebased after the existing dependency update via dependabot has been merged for this pull request to be reviewed.
  • If this pull request is for supporting a new AWS service:
    • Ensure the new AWS service changes are following the Contributing Guide section on new services, in particular that the dependency addition and initial provider support are in a separate pull request from other changes (e.g. new resources). Contributions not following this item will not be reviewed until the changes are split.
    • If this pull request is already a separate pull request from the above item, you can ignore this message.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Welcome @Isaiah-Turner 👋

It looks like this is your first Pull Request submission to the Terraform AWS Provider! If you haven’t already done so please make sure you have checked out our CONTRIBUTING guide and FAQ to make sure your contribution is adhering to best practice and has all the necessary elements in place for a successful approval.

Also take a look at our FAQ which details how we prioritize Pull Requests for inclusion.

Thanks again, and welcome to the community! 😃

@Isaiah-Turner Isaiah-Turner changed the title Add appconfig Add AWS AppConfig support Mar 9, 2021
@Isaiah-Turner Isaiah-Turner requested a review from jfirebaugh March 9, 2021 18:03
updateInput.Description = aws.String(n.(string))
}

if d.HasChange("name") {

Choose a reason for hiding this comment

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

just out of curiosity, does this handle things like can't update only create/destroy automatically? Or do you need to put in logic here to handle that?

configProfileName := aws.String(d.Get("name").(string))
configProfileDescription := aws.String(d.Get("description").(string))
locationURI := aws.String(d.Get("location_uri").(string))
retreivalRoleArn := aws.String(d.Get("retrieval_role_arn").(string))

Choose a reason for hiding this comment

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

misspelling: retrievalRoleArn

configProfileName := aws.String(d.Get("name").(string))
configProfileDescription := aws.String(d.Get("description").(string))
retreivalRoleArn := aws.String(d.Get("retrieval_role_arn").(string))
if *retreivalRoleArn == "" {

Choose a reason for hiding this comment

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

spelling: retrieval

validation.StringLenBetween(0, 1024),
),
},
"monitor": {

Choose a reason for hiding this comment

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

I think in the description this is: "monitors" plural, not singular

Copy link
Author

Choose a reason for hiding this comment

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

EnvironmentId: environmentID,
Description: environmentDescription,
Name: environmentName,
Monitors: monitorList,

Choose a reason for hiding this comment

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

is it not possible to update monitors since there's no getChange calling it?

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 still update, since d.Get("monitors") will get the current monitors. I looked at some other resources, and I think I don't need get changes. Other resources look like:

func resourceAwsDataSyncTaskUpdate(d *schema.ResourceData, meta interface{}) error {
	conn := meta.(*AWSClient).datasyncconn

	if d.HasChanges("options", "name") {
		input := &datasync.UpdateTaskInput{
			Options: expandDataSyncOptions(d.Get("options").([]interface{})),
			Name:    aws.String(d.Get("name").(string)),
			TaskArn: aws.String(d.Id()),
		}

		log.Printf("[DEBUG] Updating DataSync Task: %s", input)
		if _, err := conn.UpdateTask(input); err != nil {
			return fmt.Errorf("error creating DataSync Task: %s", err)
		}
	}

	if d.HasChange("tags") {
		o, n := d.GetChange("tags")

		if err := keyvaluetags.DatasyncUpdateTags(conn, d.Id(), o, n); err != nil {
			return fmt.Errorf("error updating DataSync Task (%s) tags: %s", d.Id(), err)
		}
	}

	return resourceAwsDataSyncTaskRead(d, meta)
}

Ill update all the resources to match that kind of update. I think GetChange is for when a field has some specific extra processing that Terraform has to do if there's a change.

@Isaiah-Turner Isaiah-Turner merged commit 605a601 into figma Mar 17, 2021
@Isaiah-Turner Isaiah-Turner deleted the add_appconfig branch March 17, 2021 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants