[Bug]: aws_sagemaker_servicecatalog_portfolio_status id attribute returns a region name instead of a portfolio id

[alixinne]

Terraform Core Version

1.3.5

AWS Provider Version

4.46.0

Affected Resource(s)

• aws_sagemaker_servicecatalog_portfolio_status

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 the aws_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

terraform {
  required_version = ">= 1.3"

  required_providers {
    aws = "4.46.0"
  }
}

# VPC info
data "aws_vpc" "default" {
}

data "aws_subnets" "default" {
  filter {
    name   = "vpc-id"
    values = [data.aws_vpc.default.id]
  }
}

# https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy
data "aws_iam_policy" "sagemaker-full-access" {
  name = "AmazonSageMakerFullAccess"
}

data "aws_iam_policy_document" "sagemaker-execution-policy" {
  statement {
    actions = ["sts:AssumeRole"]

    principals {
      type        = "Service"
      identifiers = ["sagemaker.amazonaws.com"]
    }
  }
}

# https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role
resource "aws_iam_role" "sagemaker_execution_role" {
  name               = "role-sagemaker-01"
  assume_role_policy = data.aws_iam_policy_document.sagemaker-execution-policy.json
}

resource "aws_iam_role_policy_attachment" "sagemaker_execution_role_policy" {
  role       = aws_iam_role.sagemaker_execution_role.name
  policy_arn = data.aws_iam_policy.sagemaker-full-access.arn
}

resource "aws_sagemaker_domain" "default" {
  domain_name = "my-sagemaker-domain"
  auth_mode   = "IAM"

  vpc_id     = data.aws_vpc.default.id
  subnet_ids = data.aws_subnets.default.ids

  default_user_settings {
    execution_role = aws_iam_role.sagemaker_execution_role.arn
  }
}

resource "aws_sagemaker_servicecatalog_portfolio_status" "default" {
  status = "Enabled"
}

# This part is required to enable access from the execution role to the shared portfolio ("SageMaker templates" in the project tab)
resource "aws_servicecatalog_principal_portfolio_association" "default" {
  #aws_sagemaker_servicecatalog_portfolio_status.default.id
  portfolio_id  = "port-57s6knszbkpqy" 
  principal_arn = aws_iam_role.sagemaker_execution_role.arn
}

output "portfolio_share_id" {
  description = "Enable portfolio share returned id"
  value       = aws_sagemaker_servicecatalog_portfolio_status.default.id
}

Steps to Reproduce

• Run 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

d.SetId(meta.(*conns.AWSClient).Region)

Would you like to implement a fix?

No

[github-actions]

Community Note

Voting for Prioritization

• Please vote on this issue by adding a 👍 reaction to the original post to help the community and maintainers prioritize this 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.

Volunteering to Work on This Issue

• If you are interested in working on this issue, please leave a comment.
• If this would be your first contribution, please review the contribution guide.

[alixinne]

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
}

[DrFaust92]

not a bug as sagemaker api only supports enable/disable and doest export the id in any way

[alixinne]

Even if it isn't a bug, I guess calling the output id is still surprising considering it's not the corresponding portfolio share id.

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.

[dod38fr]

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