Issue #7573 terraform crash - codepipeline

[saravanan30erd]

Fixes #7573

Changes proposed in this pull request:

• Change 1
  verifying the output before set to attr - aws_codepipeline

[bflad]

Hi @saravanan30erd 👋 Thanks for submitting this. I believe there is more at play here than just the two string pointers here that previously did not handle pointer safety correctly. I've asked for some clarification in the original issue (#7573).

It looks like in November last year, AWS CodePipeline released new cross-region functionality which added a new optional ArtifactStores attributes and made the previously required ArtifactStore attribute optional as well. The aws_codepipeline resource code is currently assuming that ArtifactStore is always passed back from the API response without its own nil check.

To properly fix the current situation leaves us in a little bit of a bind as this is new functionality, but assumptions about the old functionality are causing an unexpected crash. We'll need to decide if we want to do one of the following:

• Return an error mentioning the lack of cross-region support if ArtifactStores has elements
• Implement the multiple ArtifactsStores functionality

In either case we should properly perform a nil check for ArtifactStore itself within the flattenAwsCodePipelineArtifactStore function for good measure:

func flattenAwsCodePipelineArtifactStore(artifactStore *codepipeline.ArtifactStore) []interface{} {
	if artifactStore == nil {
		return []interface{}{}
	}
	values := map[string]interface{}{
		"location": aws.StringValue(artifactStore.Location),
		"type": aws.StringValue(artifactStore.Type),
	}
	// ...

In general, its usually helpful to find a reproducing test configuration so we can ensure we are implementing a proper fix. My best guess is that the initial fix here, while better code practice, would not actually help in this situation. Hope this makes sense.

References:

• Cross Region Pipelines #6871
• https://aws.amazon.com/about-aws/whats-new/2018/11/aws-codepipeline-now-supports-cross-region-actions/
• https://docs.aws.amazon.com/codepipeline/latest/userguide/actions-create-cross-region.html
• aws/aws-sdk-go@v1.15.73...v1.15.74#diff-01c71303770b59347b98c541781a63f5
• https://docs.aws.amazon.com/codepipeline/latest/APIReference/API_PipelineDeclaration.html

[bflad]

Performing a nil check before using the AWS Go SDK type conversion functions is redundant. They convert nil to the zero-value for the type: https://docs.aws.amazon.com/sdk-for-go/api/aws/#StringValue

[bflad]

Closing for now given the above and no additional movement within this pull request.

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