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.

Sign up for GitHub

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

Jump to bottom

Support additional CodeBuild settings, retry for IAM errors #3929

Merged
merged 8 commits into from
May 24, 2018
Merged

Support additional CodeBuild settings, retry for IAM errors #3929

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
a60862f
Retry UpdateProject similar to CreateProject since IAM is eventually …
errriclee Mar 27, 2018
20b148f
Support CodeBuild environment variable type, git clone depth, insecur…
errriclee Mar 27, 2018
afac942
Fix Sprintf vet errors not passing string
errriclee Mar 27, 2018
a61a65d
Remove unintended testing comment
errriclee Mar 27, 2018
e3c29ee
Add GHE example, document new attributes
errriclee Mar 27, 2018
914bacf
Add state migration of source attributes to new hash with defaults
errriclee Apr 2, 2018
0a75b77
Revert "Retry UpdateProject similar to CreateProject since IAM is eve…
errriclee Apr 17, 2018
da75f28
Revert "Revert "Retry UpdateProject similar to CreateProject since IA…
errriclee Apr 17, 2018
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
70 changes: 65 additions & 5 deletions aws/resource_aws_codebuild_project.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"log"
"regexp"
"strconv"
"time"

"github.com/aws/aws-sdk-go/aws"
Expand All @@ -22,6 +23,9 @@ func resourceAwsCodeBuildProject() *schema.Resource {
Update: resourceAwsCodeBuildProjectUpdate,
Delete: resourceAwsCodeBuildProjectDelete,

SchemaVersion: 2,
MigrateState: resourceAwsCodebuildMigrateState,

Schema: map[string]*schema.Schema{
"artifacts": {
Type: schema.TypeSet,
Expand Down Expand Up @@ -106,6 +110,15 @@ func resourceAwsCodeBuildProject() *schema.Resource {
Type: schema.TypeString,
Required: true,
},
"type": {
Type: schema.TypeString,
Optional: true,
ValidateFunc: validation.StringInSlice([]string{
codebuild.EnvironmentVariableTypePlaintext,
codebuild.EnvironmentVariableTypeParameterStore,
}, false),
Default: codebuild.EnvironmentVariableTypePlaintext,
},
},
},
},
Expand Down Expand Up @@ -184,6 +197,15 @@ func resourceAwsCodeBuildProject() *schema.Resource {
codebuild.SourceTypeGithubEnterprise,
}, false),
},
"git_clone_depth": {
Type: schema.TypeInt,
Optional: true,
ValidateFunc: validation.IntAtLeast(0),
},
"insecure_ssl": {
Type: schema.TypeBool,
Optional: true,
},
},
},
Required: true,
Expand Down Expand Up @@ -373,6 +395,10 @@ func expandProjectEnvironment(d *schema.ResourceData) *codebuild.ProjectEnvironm
projectEnvironmentVar.Value = &v
}

if v := config["type"].(string); v != "" {
projectEnvironmentVar.Type = &v
}

projectEnvironmentVariables = append(projectEnvironmentVariables, projectEnvironmentVar)
}

Expand Down Expand Up @@ -408,11 +434,15 @@ func expandProjectSource(d *schema.ResourceData) codebuild.ProjectSource {
sourceType := data["type"].(string)
location := data["location"].(string)
buildspec := data["buildspec"].(string)
gitCloneDepth := aws.Int64(int64(data["git_clone_depth"].(int)))
insecureSsl := aws.Bool(data["insecure_ssl"].(bool))

projectSource = codebuild.ProjectSource{
Type: &sourceType,
Location: &location,
Buildspec: &buildspec,
Type: &sourceType,
Location: &location,
Buildspec: &buildspec,
GitCloneDepth: gitCloneDepth,
InsecureSsl: insecureSsl,
}

if v, ok := data["auth"]; ok {
Expand Down Expand Up @@ -527,7 +557,22 @@ func resourceAwsCodeBuildProjectUpdate(d *schema.ResourceData, meta interface{})
// But its a slice of pointers so if not set for every update, they get removed.
params.Tags = tagsFromMapCodeBuild(d.Get("tags").(map[string]interface{}))

_, err := conn.UpdateProject(params)
err := resource.Retry(5*time.Minute, func() *resource.RetryError {
Copy link
Contributor

Choose a reason for hiding this comment

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

We tend to only retry for 1 minute unless there is heavy evidence it requires longer. Can you please also move this to its own PR so it can get merged separately of the other changes? Reference: #4233

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now #4238

var err error

_, err = conn.UpdateProject(params)
if err != nil {
// Work around eventual consistency of IAM
if isAWSErr(err, "InvalidInputException", "CodeBuild is not authorized to perform") {
return resource.RetryableError(err)
}

return resource.NonRetryableError(err)
}

return nil

})

if err != nil {
return fmt.Errorf(
Expand Down Expand Up @@ -623,6 +668,14 @@ func flattenAwsCodeBuildProjectSource(source *codebuild.ProjectSource) []interfa
m["location"] = *source.Location
}

if source.GitCloneDepth != nil {
m["git_clone_depth"] = *source.GitCloneDepth
}

if source.InsecureSsl != nil {
m["insecure_ssl"] = *source.InsecureSsl
}

l[0] = m

return l
Expand Down Expand Up @@ -668,7 +721,7 @@ func resourceAwsCodeBuildProjectEnvironmentHash(v interface{}) int {
for _, e := range environmentVariables {
if e != nil { // Old statefiles might have nil values in them
ev := e.(map[string]interface{})
buf.WriteString(fmt.Sprintf("%s:%s-", ev["name"].(string), ev["value"].(string)))
buf.WriteString(fmt.Sprintf("%s:%s:%s-", ev["name"].(string), ev["type"].(string), ev["value"].(string)))
}
}

Expand All @@ -686,6 +739,12 @@ func resourceAwsCodeBuildProjectSourceHash(v interface{}) int {
if v, ok := m["location"]; ok {
buf.WriteString(fmt.Sprintf("%s-", v.(string)))
}
if v, ok := m["git_clone_depth"]; ok {
buf.WriteString(fmt.Sprintf("%s-", strconv.Itoa(v.(int))))
}
if v, ok := m["insecure_ssl"]; ok {
buf.WriteString(fmt.Sprintf("%s-", strconv.FormatBool(v.(bool))))
}

return hashcode.String(buf.String())
}
Expand All @@ -711,6 +770,7 @@ func environmentVariablesToMap(environmentVariables []*codebuild.EnvironmentVari
item := map[string]interface{}{}
item["name"] = *env.Name
item["value"] = *env.Value
item["type"] = *env.Type
envVariables = append(envVariables, item)
}
}
Expand Down
54 changes: 54 additions & 0 deletions aws/resource_aws_codebuild_project_migrate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"log"
"strings"

"github.com/hashicorp/terraform/helper/schema"
"github.com/hashicorp/terraform/terraform"
)

Expand All @@ -14,6 +15,9 @@ func resourceAwsCodebuildMigrateState(
case 0:
log.Println("[INFO] Found AWS Codebuild State v0; migrating to v1")
return migrateCodebuildStateV0toV1(is)
case 1:
log.Println("[INFO] Found AWS Codebuild State v1; migrating to v2")
return migrateCodebuildStateV1toV2(is)
default:
return is, fmt.Errorf("Unexpected schema version: %d", v)
}
Expand All @@ -34,3 +38,53 @@ func migrateCodebuildStateV0toV1(is *terraform.InstanceState) (*terraform.Instan
log.Printf("[DEBUG] Attributes after migration: %#v", is.Attributes)
return is, nil
}

func migrateCodebuildStateV1toV2(is *terraform.InstanceState) (*terraform.InstanceState, error) {
if is.Empty() {
log.Println("[DEBUG] Empty InstanceState; nothing to migrate.")
return is, nil
}

log.Printf("[DEBUG] Attributes before migration v1: %#v", is.Attributes)

prefix := "source"
entity := resourceAwsCodeBuildProject()

// Read old keys
reader := &schema.MapFieldReader{
Schema: entity.Schema,
Map: schema.BasicMapReader(is.Attributes),
}
result, err := reader.ReadField([]string{prefix})
if err != nil {
return nil, err
}

oldKeys, ok := result.Value.(*schema.Set)
if !ok {
return nil, fmt.Errorf("Got unexpected value from state: %#v", result.Value)
}

// Delete old keys
for k := range is.Attributes {
if strings.HasPrefix(k, fmt.Sprintf("%s.", prefix)) {
delete(is.Attributes, k)
}
}

// We just need the defaults for the new keys, no custom values needed

// Write new keys
writer := schema.MapFieldWriter{
Schema: entity.Schema,
}
if err := writer.WriteField([]string{prefix}, oldKeys); err != nil {
return is, err
}
for k, v := range writer.Map() {
is.Attributes[k] = v
}

log.Printf("[DEBUG] Attributes after migration v2: %#v", is.Attributes)
return is, nil
}
73 changes: 68 additions & 5 deletions aws/resource_aws_codebuild_project_migrate_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package aws

import (
"reflect"
"testing"

"github.com/hashicorp/terraform/terraform"
Expand All @@ -11,7 +12,7 @@ func TestAWSCodebuildMigrateState(t *testing.T) {
StateVersion int
ID string
Attributes map[string]string
Expected string
Expected map[string]string
Meta interface{}
}{
"v0_1": {
Expand All @@ -21,7 +22,11 @@ func TestAWSCodebuildMigrateState(t *testing.T) {
"description": "some description",
"timeout": "5",
},
Expected: "5",
Expected: map[string]string{
"description": "some description",
"timeout": "5",
"build_timeout": "5",
},
},
"v0_2": {
StateVersion: 0,
Expand All @@ -30,7 +35,65 @@ func TestAWSCodebuildMigrateState(t *testing.T) {
"description": "some description",
"build_timeout": "5",
},
Expected: "5",
Expected: map[string]string{
"description": "some description",
"build_timeout": "5",
},
},
"v1_1": {
StateVersion: 1,
ID: "tf-testing-file",
Attributes: map[string]string{
"description": "some description",
"source.1060593600.auth.2706882902.type": "OAUTH",
"source.1060593600.type": "GITHUB",
"source.1060593600.buildspec": "",
"source.#": "1",
"source.1060593600.location": "https://github.com/hashicorp/packer.git",
"source.1060593600.auth.2706882902.resource": "FAKERESOURCE1",
"source.1060593600.auth.#": "1",
},
// added defaults
Expected: map[string]string{
"description": "some description",
"source.#": "1",
"source.3680370172.type": "GITHUB",
"source.3680370172.buildspec": "",
"source.3680370172.location": "https://github.com/hashicorp/packer.git",
"source.3680370172.git_clone_depth": "0",
"source.3680370172.insecure_ssl": "false",
"source.3680370172.auth.#": "1",
"source.3680370172.auth.2706882902.resource": "FAKERESOURCE1",
"source.3680370172.auth.2706882902.type": "OAUTH",
},
},
"v1_noop": {
StateVersion: 1,
ID: "tf-testing-file",
Attributes: map[string]string{
"description": "some description",
"source.#": "1",
"source.3680370172.type": "GITHUB",
"source.3680370172.buildspec": "",
"source.3680370172.location": "https://github.com/hashicorp/packer.git",
"source.3680370172.git_clone_depth": "0",
"source.3680370172.insecure_ssl": "false",
"source.3680370172.auth.#": "1",
"source.3680370172.auth.2706882902.resource": "FAKERESOURCE1",
"source.3680370172.auth.2706882902.type": "OAUTH",
},
Expected: map[string]string{
"description": "some description",
"source.#": "1",
"source.3680370172.type": "GITHUB",
"source.3680370172.buildspec": "",
"source.3680370172.location": "https://github.com/hashicorp/packer.git",
"source.3680370172.git_clone_depth": "0",
"source.3680370172.insecure_ssl": "false",
"source.3680370172.auth.#": "1",
"source.3680370172.auth.2706882902.resource": "FAKERESOURCE1",
"source.3680370172.auth.2706882902.type": "OAUTH",
},
},
}

Expand All @@ -46,8 +109,8 @@ func TestAWSCodebuildMigrateState(t *testing.T) {
t.Fatalf("bad: %s, err: %#v", tn, err)
}

if is.Attributes["build_timeout"] != tc.Expected {
t.Fatalf("Bad build_timeout migration: %s\n\n expected: %s", is.Attributes["build_timeout"], tc.Expected)
if !reflect.DeepEqual(is.Attributes, tc.Expected) {
t.Fatalf("Bad migration: %s\n\n expected: %s", is.Attributes, tc.Expected)
}
}
}
Loading