-
Notifications
You must be signed in to change notification settings - Fork 8.9k
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
Feature/app fabric app authorization #37468
base: main
Are you sure you want to change the base?
Feature/app fabric app authorization #37468
Conversation
Community NoteVoting for Prioritization
For Submitters
|
After fixing the tests: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent code, job well done. Just some minor edits to follow.
// SPDX-License-Identifier: MPL-2.0 | ||
|
||
package appfabric | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks Good
|
||
resource "aws_appfabric_app_authorization" "test" { | ||
app_bundle_identifier = aws_appfabric_app_bundle.arn | ||
app = "TERRAFORMCLOUD" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a valid value? As per this doco - looks like itts noto a valid value -- https://docs.aws.amazon.com/appfabric/latest/api/API_CreateAppAuthorization.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes its Valid the docs doenst have all the possible names that was tested
|
||
|
||
resource "aws_appfabric_app_authorization" "test" { | ||
app_bundle_identifier = aws_appfabric_app_bundle.arn |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is the resource for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is still on creation the tests will be updated with the team that is working on that resource will make a pull request
} | ||
} | ||
|
||
func testAccAppAuthorizationConfig_basic() string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The return requires formatting..
```terraform | ||
resource "aws_appfabric_app_authorization" "example" { | ||
app = "TERRAFORMCLOUD" | ||
app_bundle_identifier = aws_appfabric_app_bundle.arn |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
include a resource for app_bundle.arn and the formatting is incorrect. You might want to consider aws_appfabric_app_bundle.test.arn
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes the resource is not ready yet waiting for the pull request
Description
This PR adds a resource for aws_appfabric_app_authorization
https://docs.aws.amazon.com/appfabric/latest/api/API_CreateAppAuthorization.html
Relations
Relates #34549
References
Output from Acceptance Testing