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

provider/aws: repository_url is computed for aws_ecr_repository #5524

Merged
merged 1 commit into from
Mar 9, 2016

Conversation

stack72
Copy link
Contributor

@stack72 stack72 commented Mar 8, 2016

As requested in #5518, we can compute the repository_url for ECR using a known format:

https://aws_account_id.dkr.ecr.region.amazonaws.com/repositoryName
ake testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSEcrRepository_basic'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /vendor/)
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSEcrRepository_basic -timeout 120m
=== RUN   TestAccAWSEcrRepository_basic
--- PASS: TestAccAWSEcrRepository_basic (6.97s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    6.983s

FYI, I didn't want to hardcode an account_id into the tests :)

But when I run this locally, I get the following output:

STATE:

aws_ecr_repository.default:
  ID = foo-repository-terraform
  arn = arn:aws:ecr:us-east-1:<ID>:repository/foo-repository-terraform
  name = foo-repository-terraform
  registry_id = <ID>
  repository_url = https://<ID>.dkr.ecr.us-east-1.amazonaws.com/foo-repository-terraform

return nil
}

func buildRepositoryUrl(repo *ecr.Repository, meta interface{}) string {
Copy link
Member

Choose a reason for hiding this comment

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

It would be nice to make this function less greedy in terms of arguments. e.g.

func buildRepositoryUrl(repo *ecr.Repository, region string) string {

@radeksimko
Copy link
Member

Besides the nitpick with arguments this LGTM.

@stack72
Copy link
Contributor Author

stack72 commented Mar 9, 2016

@radeksimko made the change as requested. Makes sense :) Will merge when it goes green!

stack72 added a commit that referenced this pull request Mar 9, 2016
provider/aws: `repository_url` is computed for `aws_ecr_repository`
@stack72 stack72 merged commit 4601d37 into hashicorp:master Mar 9, 2016
@stack72 stack72 deleted the f-aws-ecr-repository_name branch March 9, 2016 09:38
Copy link

@jim3ma jim3ma left a comment

Choose a reason for hiding this comment

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

please removing the https:// part, the https:// will cause 500 for ecs agent

return nil
}

func buildRepositoryUrl(repo *ecr.Repository, region string) string {
return fmt.Sprintf("https://%s.dkr.ecr.%s.amazonaws.com/%s", *repo.RegistryId, region, *repo.RepositoryName)
Copy link

Choose a reason for hiding this comment

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

Why https ? just return fmt.Sprintf("%s.dkr.ecr.%s.amazonaws.com/%s", *repo.RegistryId, region, *repo.RepositoryName) is okay!

Choose a reason for hiding this comment

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

I have the same issue. I'd like to use this in a container definition template and the HTTPS breaks it.

@ghost
Copy link

ghost commented Apr 18, 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 18, 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.

4 participants