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

[New Resource] aws_servicecatalogappregistry_application #36277

Merged
merged 15 commits into from Mar 26, 2024

Conversation

fatbasstard
Copy link
Contributor

@fatbasstard fatbasstard commented Mar 8, 2024

Description

This PR adds a new resource: aws_servicecatalogappregistry_application

In AWS this is now also launched as myApplications(announcement, documentation)

So the naming can be "confusing", since the application itself technically is a "Service Catalog App Registry Application" (and I wanted to stick with the naming convention). But to somehow make clear it's the base of myApplication added it as a note to the documentation (so it's clear and findable if you search for TF and myApplications)

Relations

Closes #34702
Closes #35255.

References

PR #35255 was created but I decided to recreate it for multiple reasons: This one uses Terraform Plugin SDK V2, naming convention is "off" and there has been no reaction from the author (and I want/need this feature to get insights on our platform).

There might be more options to the resource but this PR enables the core functionality to start implementing the myApplications feature.

Also planning to add a datasource later (in a separate PR)

Output from Acceptance Testing

% make testacc TESTS=TestAccServiceCatalogAppRegistryApplication_ PKG=servicecatalogappregistry 

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/servicecatalogappregistry/... -v -count 1 -parallel 20 -run='TestAccServiceCatalogAppRegistryApplication_'  -timeout 360m
=== RUN   TestAccServiceCatalogAppRegistryApplication_basic
=== PAUSE TestAccServiceCatalogAppRegistryApplication_basic
=== RUN   TestAccServiceCatalogAppRegistryApplication_disappears
=== PAUSE TestAccServiceCatalogAppRegistryApplication_disappears
=== CONT  TestAccServiceCatalogAppRegistryApplication_basic
=== CONT  TestAccServiceCatalogAppRegistryApplication_disappears
--- PASS: TestAccServiceCatalogAppRegistryApplication_disappears (7.71s)
--- PASS: TestAccServiceCatalogAppRegistryApplication_basic (8.68s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/servicecatalogappregistry	11.876s
...

Copy link

github-actions bot commented Mar 8, 2024

Community Note

Voting for Prioritization

  • Please vote on this pull request by adding a 👍 reaction to the original post to help the community and maintainers prioritize this pull request.
  • Please see our prioritization guide for information on how we prioritize.
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.

For Submitters

  • Review the contribution guide relating to the type of change you are making to ensure all of the necessary steps have been taken.
  • For new resources and data sources, use skaff to generate scaffolding with comments detailing common expectations.
  • Whether or not the branch has been rebased will not impact prioritization, but doing so is always a welcome surprise.

@github-actions github-actions bot added size/XL Managed by automation to categorize the size of a PR. documentation Introduces or discusses updates to documentation. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. generators Relates to code generators. service/servicecatalogappregistry Issues and PRs that pertain to the servicecatalogappregistry service. labels Mar 8, 2024
@terraform-aws-provider terraform-aws-provider bot added the needs-triage Waiting for first response or review from a maintainer. label Mar 8, 2024
@fatbasstard fatbasstard force-pushed the f-aws_servicecatalog_application branch 2 times, most recently from 9c9a55e to 205faa7 Compare March 8, 2024 18:42
@fatbasstard fatbasstard force-pushed the f-aws_servicecatalog_application branch from 205faa7 to 3374025 Compare March 8, 2024 19:09
@justinretzolk justinretzolk added new-resource Introduces a new resource. and removed needs-triage Waiting for first response or review from a maintainer. labels Mar 8, 2024
@hbjydev
Copy link

hbjydev commented Mar 11, 2024

@fatbasstard Could you look into implementing a data "aws_servicecatalogappregistry_application" resource too? It would be really useful for people like my team who deploy one application with multiple TF repos.

@fatbasstard
Copy link
Contributor Author

fatbasstard commented Mar 11, 2024

@fatbasstard Could you look into implementing a data "aws_servicecatalogappregistry_application" resource too? It would be really useful for people like my team who deploy one application with multiple TF repos.

Hi, definitely. But plan is to make that a separate/next PR. I'm fairly new in this codebase so want to be sure this is up to par. When it is through I'll make more changes (like a datasource)

@hbjydev
Copy link

hbjydev commented Mar 14, 2024

Awesome! Look forward to it.

@fatbasstard
Copy link
Contributor Author

@ewbankkit Hi!

My previous 2 PR's went trough really fast, this one has been open for a while. Any specific reason why it's not processed? Anything I can do to speed up the process?

@fatbasstard
Copy link
Contributor Author

@YakDriver @DrFaust92 @ewbankkit @johnsonaj @jar-b Hi all, can somebody take a look at this PR? We're really looking forward to this feature ever since it was announced in November. Any feedback would be very appreciated

@jar-b jar-b self-assigned this Mar 26, 2024
@terraform-aws-provider terraform-aws-provider bot added the prioritized Part of the maintainer teams immediate focus. To be addressed within the current quarter. label Mar 26, 2024
Copy link
Member

@jar-b jar-b left a comment

Choose a reason for hiding this comment

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

LGTM 🎉

% make testacc PKG=servicecatalogappregistry TESTS=TestAccServiceCatalogAppRegistryApplication_
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/servicecatalogappregistry/... -v -count 1 -parallel 20 -run='TestAccServiceCatalogAppRegistryApplication_'  -timeout 360m

--- PASS: TestAccServiceCatalogAppRegistryApplication_disappears (11.48s)
--- PASS: TestAccServiceCatalogAppRegistryApplication_basic (13.42s)
--- PASS: TestAccServiceCatalogAppRegistryApplication_update (28.16s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/servicecatalogappregistry  32.857s

@jar-b
Copy link
Member

jar-b commented Mar 26, 2024

Thanks for your contribution, @fatbasstard! 👍

@fatbasstard
Copy link
Contributor Author

Thanks for handling the PR!

Looking forward to making more contributions

@jar-b jar-b merged commit 50a4be7 into hashicorp:main Mar 26, 2024
44 checks passed
@github-actions github-actions bot added this to the v5.43.0 milestone Mar 26, 2024
@github-actions github-actions bot removed the prioritized Part of the maintainer teams immediate focus. To be addressed within the current quarter. label Mar 28, 2024
Copy link

This functionality has been released in v5.43.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

@kevindesao
Copy link

Hello,
I was really excited to try this out but it turns out I'm not able to make it work smoothly.

The creation of the resource worked fine but the implementation of the ARN as a tag fails to link the resource to the app.

The doc is mentioning the following:

resource "aws_servicecatalogappregistry_application" "example" {
  name = "example-app"
}

resource "aws_s3_bucket" "bucket" {
  bucket = "example-bucket"

  tags = {
    awsApplication = aws_servicecatalogappregistry_application.example.arn
  }
}

But by looking into the created myApp in the console the tags is not simply the ARN of the myApplication resources.

  • ARN myApplication : arn:aws:servicecatalog:<region>:<account-id>:/applications/<ramdom_ARN_ID>
  • Application tag value: arn:aws:resource-groups:<region>:<account-id>:group/<myApplication_name>/<ramdom_ARN_ID>

Tag Value tried unsuccessfully:

  • simply the myApplication id : <ramdom_ARN_ID>
  • The ARN arn:aws:servicecatalog:<region>:<account-id>:/applications/<ramdom_ARN_ID>

So to resume, I think a new attribute is required:

SOLUTION: In the meantime the ARN can be reconstructed like this: (Tested)

  common_tags = {
    awsApplication = "arn:aws:resource-groups:${data.aws_region.myApplication.name}:${data.aws_caller_identity.myApplication.id}:group/${aws_servicecatalogappregistry_application.this.name}/${aws_servicecatalogappregistry_application.this.id}"
  }

@kevindesao
Copy link

Good afternoon @fatbasstard @jar-b !

Really hyped by this new feature thanks for implementing it, I have created a issue following my precedent feedback.
If you can have a look while doing the data module it would be lovely :)

But in the meantime since it works with the crafted tags thanks again !

Copy link

I'm going to lock this pull request 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 related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Introduces or discusses updates to documentation. generators Relates to code generators. new-resource Introduces a new resource. service/servicecatalogappregistry Issues and PRs that pertain to the servicecatalogappregistry service. size/XL Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[New Resource]: Resource for aws_servicecatalog_application
5 participants