-
Notifications
You must be signed in to change notification settings - Fork 9.2k
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
[Bug]: aws_sagemaker_servicecatalog_portfolio_status id attribute returns a region name instead of a portfolio id #28348
Comments
Community NoteVoting for Prioritization
Volunteering to Work on This Issue
|
If you happen to have this issue as well, this is the workaround we are currently using: resource "aws_sagemaker_servicecatalog_portfolio_status" "default" {
status = "Enabled"
}
# aws_sagemaker_servicecatalog_portfolio_status doesn't return the actual
# portfolio id for now, so we need to fetch it somehow. Instead of depending on
# an external boto3-based script, we can rely on the available aws cli to pull
# the ID using the external data source.
data "external" "portfolio_shares" {
# depends_on is required because the
# aws_sagemaker_servicecatalog_portfolio_status resource is responsible for
# creating the portfolio share.
depends_on = [
aws_sagemaker_servicecatalog_portfolio_status.default
]
program = [
"aws", "servicecatalog", "list-accepted-portfolio-shares",
"--output", "json",
"--query", "PortfolioDetails[?ProviderName==`Amazon SageMaker`] | [0]"
]
}
resource "aws_servicecatalog_principal_portfolio_association" "default" {
portfolio_id = data.external.portfolio_shares.result.Id
principal_arn = aws_iam_role.sagemaker_execution_role.arn
} |
not a bug as sagemaker api only supports enable/disable and doest export the id in any way |
Even if it isn't a bug, I guess calling the output Maybe the provider could call https://docs.aws.amazon.com/sdk-for-go/api/service/servicecatalog/#ServiceCatalog.ListAcceptedPortfolioShares to retrieve the generated portfolio share id to return it instead? This would only be a workaround until the SageMaker API gets updated to return that id directly when calling the enable service catalog portfolio API. |
Another way out would be to be able to search the current list of portfolios with a dedicated data source. There's an open issue to get that feature: #20719 |
Terraform Core Version
1.3.5
AWS Provider Version
4.46.0
Affected Resource(s)
Expected Behavior
This issue is a follow-up to the recent addition of the
aws_sagemaker_servicecatalog_portfolio_status
resource.As pointed out in this comment, the portfolio share created by the enable sagemaker service catalog portfolio API call may need to be associated with a principal. This is needed to access the resources defined inside the shared portfolio from a customer managed execution role for example, as noted in the provided config below.
However, the (maybe improperly-named?)
id
attribute of this resource does not return the created portfolio share ID, only the current provider region which is not of much use.Since there is no
aws_servicecatalog_portfolio_share
data source, we have no way of finding out the details of the created portfolio share, which would require explicit dependencies anyways. Should theaws_sagemaker_servicecatalog_portfolio_status
id attribute instead return the portfolio share ID?Actual Behavior
The
aws_sagemaker_servicecatalog_portfolio_status
id attribute returns the current region name instead of a portfolio share ID.Relevant Error/Panic Output Snippet
No response
Terraform Configuration Files
Steps to Reproduce
terraform apply
with the above snippet.Note that aws_servicecatalog_principal_portfolio_association uses a hardcoded ID, discovered through
aws servicecatalog list-accepted-portfolio-shares
.This ID should be returned by the aws_servicecatalog_principal_portfolio_association resource instead.
Debug Output
No response
Panic Output
No response
Important Factoids
No response
References
Original issue: #23917
PR that introduced the resource: #27548
See the offending line in the corresponding resource:
terraform-provider-aws/internal/service/sagemaker/servicecatalog_portfolio_status.go
Line 47 in 22c1e5c
Would you like to implement a fix?
No
The text was updated successfully, but these errors were encountered: