Skip to content

Conversation

@stephybun
Copy link
Member

Description

  • Calls Configure in the ListResource RPC
  • Removes a ListResource call in the ValidateListResourceConfig

Rollback Plan

  • If a change needs to be reverted, we will roll out an update to the code within 7 days.

Changes to Security Controls

None

@stephybun stephybun requested a review from a team as a code owner July 24, 2025 11:35
@stephybun stephybun force-pushed the b/minor-list-fixes branch from 8ac8872 to 575ddd7 Compare July 30, 2025 12:34
diagsStream.Results = list.NoListResults
}

fwStream.Results = processListResults(req, stream.Results, diagsStream.Results)
Copy link
Member Author

Choose a reason for hiding this comment

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

@dbanck in order to capture any warning diagnostics that providers might set during the Configure call on line 115 above there might be cases where we need to append an empty ListResult with only a warning diagnostic in it to the stream.

How would this be handled by Terraform and would this be problematic?

Copy link
Member

Choose a reason for hiding this comment

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

In the current implementation we stop on errors, but continue on warnings.

But we also raise an error when a result has no identity.

https://github.com/hashicorp/terraform/blob/dec0edfd5afff7169f94424aae0590dddfddee8a/internal/plugin/grpc_provider.go#L1358-L1362

Copy link
Member

Choose a reason for hiding this comment

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

So appending a result with only a warning will still result in an error in Terraform, because the identity is missing

Copy link
Member

@austinvalle austinvalle left a comment

Choose a reason for hiding this comment

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

LGTM, we can add some tests later when we have a plan on how we want to handle those warning diagnostics 🚀

@stephybun stephybun force-pushed the b/minor-list-fixes branch from 1b63303 to 82f4776 Compare July 31, 2025 13:19
@stephybun stephybun merged commit 7633960 into main Jul 31, 2025
37 checks passed
@stephybun stephybun deleted the b/minor-list-fixes branch July 31, 2025 13:25
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 30, 2025
@austinvalle austinvalle added this to the v1.16.0 milestone Sep 16, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants