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

AWS: codecommit set default_branch only if defined #5904

Merged

Conversation

keymon
Copy link
Contributor

@keymon keymon commented Mar 29, 2016

Do not try to update the AWS codecommit repository default branch if the resource definition does not set it or is an empty string.

Fixes #5641

Note: Not sure if this is the best approach to handle the issue, as terraform reports changes all time. But it does not crash. Comments?

@@ -104,7 +104,7 @@ func resourceAwsCodeCommitRepositoryCreate(d *schema.ResourceData, meta interfac
func resourceAwsCodeCommitRepositoryUpdate(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).codecommitconn

if d.HasChange("default_branch") {
if d.HasChange("default_branch") && len(d.Get("default_branch").(string)) > 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @keymon

Would we check for a length here or a non-empty string?

P

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure what you mean, because length bigger than 0 means the same than checking non-empty string.

Anyway, checking other code in terraform I found other examples where they use the method d.GetOK(), so I refactored it to use that.

@keymon
Copy link
Contributor Author

keymon commented Mar 30, 2016

I also added a commit to check the remote repository before updating the default branch. If the default branch remotely is nil, that means that the repository has no branches and any branch update will fail.

Effectively this means that default_branch parameter will only apply if there are any branches in the remote repository. This solves the issue of creating a new repository with default_branch defined.

@stack72
Copy link
Contributor

stack72 commented Mar 30, 2016

@keymon thanks for the changes

Will run some tests on this now before the merge :)

@keymon keymon changed the title AWS: codecommit set default_branch only if defined [WIP] AWS: codecommit set default_branch only if defined Mar 30, 2016
@keymon
Copy link
Contributor Author

keymon commented Mar 30, 2016

@stack72 Hi. Thx, I have just added some tests around this.

I wanted to test the case of creating a branch, but the only way to create a branch is using git itself to push. That is more complicated, I need to create credentials, a user, and depend on external commands.

Are happy with this implementation and without a full test?

@stack72
Copy link
Contributor

stack72 commented Mar 30, 2016

@keymon thanks for adding the tests :) These are a useful addition and will be good!

Do not try to update or reaad the AWS codecommit repository default branch if
the resource definition is not set it or is an empty string.

Fixes hashicorp#5641
In AWS codecommit the default branch must have a value unless there are
no branches created, in which case it is not possible to set it to any value.

We query the existing branches and do not update the default branch
if there are none defined remotely.

This solves the issue of the initial creation of the repository with a
resource with `default_branch` defined.
The provider should, when working on a new repository without branches:
 * Able to create a new repository even with default_branch defined.
 * Able to create a new repository without default_branch, and do not fail
   if default_branch is defined.
@keymon
Copy link
Contributor Author

keymon commented Mar 30, 2016

@stack72 great, thank you.

I will do a last update, to list the remote branches instead checking the remote default branch, and remove the [WIP].

@keymon keymon force-pushed the bugfix/5661_fix_codecommit_default_branch branch from dce658b to 29c9b84 Compare March 30, 2016 15:43
@keymon keymon changed the title [WIP] AWS: codecommit set default_branch only if defined AWS: codecommit set default_branch only if defined Mar 30, 2016
@stack72
Copy link
Contributor

stack72 commented Mar 30, 2016

@keymon just give me a shout when you want me to re-run the tests pending merge

@keymon
Copy link
Contributor Author

keymon commented Mar 30, 2016

@stack72 Please, go ahead, I won't touch it again. 👍 😄

@stack72
Copy link
Contributor

stack72 commented Mar 30, 2016

Thanks @keymon - tests look great :)

TF_LOG=1 make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSCodeCommitRepository' 2>~/tf.log
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /vendor/)
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSCodeCommitRepository -timeout 120m
=== RUN   TestAccAWSCodeCommitRepository_basic
--- PASS: TestAccAWSCodeCommitRepository_basic (7.20s)
=== RUN   TestAccAWSCodeCommitRepository_withChanges
--- PASS: TestAccAWSCodeCommitRepository_withChanges (8.84s)
=== RUN   TestAccAWSCodeCommitRepository_create_default_branch
--- PASS: TestAccAWSCodeCommitRepository_create_default_branch (5.36s)
=== RUN   TestAccAWSCodeCommitRepository_create_and_update_default_branch
--- PASS: TestAccAWSCodeCommitRepository_create_and_update_default_branch (10.29s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    31.703s

@stack72 stack72 merged commit 53434ae into hashicorp:master Mar 30, 2016
@keymon
Copy link
Contributor Author

keymon commented Mar 30, 2016

Thx for merging.

Only FYI and reference. I paste here some "manual acceptance tests cases" which I did not include, as commented, due the overhead of the required scaffolding (git, credentials, etc).

Given this config:

provider "aws" {
  region = "us-east-1"
}

variable "git_rsa_id_pub" {
  description = "Public SSH key for the git user"
}

variable "default_branch" {
  description = "Default Branch for the repo"
  default = "master"
}

resource "aws_codecommit_repository" "myrepo" {
  provider = "aws"
  repository_name = "myrepo"
  description = "Test default_branch bug"
  default_branch = "${var.default_branch}"
}

resource "aws_iam_user" "mygituser" {
  name = "mygituser"
}

resource "aws_iam_user_ssh_key" "mygituser" {
  username = "${aws_iam_user.mygituser.name}"
  encoding = "PEM"
  public_key = "${var.git_rsa_id_pub}"
}

resource "aws_iam_policy" "my_aws_codecommit_full_access" {
  name = "MyAWSCodeCommitFullAccess"
  description = "Provides full access to AWS CodeCommit via the AWS Management Console"
  policy = <<EOT
{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Effect": "Allow",
            "Action": [
                "codecommit:*"
            ],
            "Resource": "*"
        }
    ]
}
EOT
}

resource "aws_iam_policy_attachment" "manage_ec2" {
  name = "ManageEC2"
  users = ["${aws_iam_user.mygituser.name}"]
  policy_arn = "${aws_iam_policy.my_aws_codecommit_full_access.arn}"
}

output "myrepo_url" {
    value = "${aws_codecommit_repository.myrepo.clone_url_ssh}"
}
output "mygituser_ssh_id" {
    value = "${aws_iam_user_ssh_key.mygituser.ssh_public_key_id}"
}

output "myrepo_fullurl" {
  # convert the ssh:// url to a scp like connect string and add the git user
  value = "ssh://${aws_iam_user_ssh_key.mygituser.ssh_public_key_id}@${replace(aws_codecommit_repository.myrepo.clone_url_ssh, "/^ssh://([^/]+)//", "$1/")}"
}
# Should be able to create a repo
terraform apply -var git_rsa_id_pub="$(< ~/.ssh/id_rsa.pub)"  -var default_branch=master

# Should clone and push
rm -rf myrepo/ && git clone ssh://APKAJFNQFNTDJCBIOQFA@git-codecommit.us-east-1.amazonaws.com/v1/repos/myrepo
(cd myrepo && git commit -m "first commit" --allow-empty && git push origin master:master )

# Should be able to update without changing default branch
terraform apply -var git_rsa_id_pub="$(< ~/.ssh/id_rsa.pub)"  -var default_branch=master

# Should fail if default_branch does not exist
terraform apply -var git_rsa_id_pub="$(< ~/.ssh/id_rsa.pub)"  -var default_branch=non_existent

# Should be able to push a different branch
rm -rf myrepo/ && git clone ssh://APKAJFNQFNTDJCBIOQFA@git-codecommit.us-east-1.amazonaws.com/v1/repos/myrepo
(cd myrepo && git commit -m "first commit" --allow-empty && git push origin master:other_branch )

# and should be able to change the default branch to a different branch
terraform apply -var git_rsa_id_pub="$(< ~/.ssh/id_rsa.pub)"  -var default_branch=other_branch

@stack72
Copy link
Contributor

stack72 commented Mar 30, 2016

Nice! This is a good indication of the fact it is working :)

@dcarley
Copy link
Contributor

dcarley commented Mar 31, 2016

Thanks @keymon and @stack72 👏

dcarley added a commit to alphagov/paas-docker-cloudfoundry-tools that referenced this pull request Apr 29, 2016
This isn't as big a jump as this individual commit suggests because we were
already using a pre-release version of 0.6.15 in 57cc1a9. We are upgrading
so that we no longer need to build from source to benefit from these fixes:

- hashicorp/terraform#5774
- hashicorp/terraform#5904

The CHANGELOG doesn't mention any backwards-incompatible changes. New
checksum has been taken from:

- https://releases.hashicorp.com/terraform/0.6.15/terraform_0.6.15_SHA256SUMS
dcarley added a commit to alphagov/paas-docker-cloudfoundry-tools that referenced this pull request Apr 29, 2016
This isn't as big a jump as this individual commit suggests because we were
already using a pre-release version of 0.6.15 in 57cc1a9. We are upgrading
so that we no longer need to build from source to benefit from these fixes:

- hashicorp/terraform#5774
- hashicorp/terraform#5904

The CHANGELOG doesn't mention any backwards-incompatible changes. New
checksum has been taken from:

- https://releases.hashicorp.com/terraform/0.6.15/terraform_0.6.15_SHA256SUMS
@ghost
Copy link

ghost commented Apr 27, 2020

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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Set default branch then apply: get a validation error
3 participants