Skip to content

Conversation

DanielMSchmidt
Copy link
Contributor

@DanielMSchmidt DanielMSchmidt commented Oct 16, 2025

Fixes #37780

Target Release

1.14.x

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

Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.

CHANGELOG entry

  • This change is user-facing and I added a changelog entry.
  • This change is not user-facing.

@DanielMSchmidt DanielMSchmidt requested a review from a team as a code owner October 16, 2025 09:27
@DanielMSchmidt DanielMSchmidt added no-changelog-needed Add this to your PR if the change does not require a changelog entry 1.14-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged labels Oct 16, 2025
matejrisek
matejrisek previously approved these changes Oct 16, 2025
Copy link
Member

@matejrisek matejrisek 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 don't allow depends_on on actions because their ordering is depending on the resource
// that triggers them, therefore users should use a depends_on on the resource instead.

if ref != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we checking for this during configload? I certainly didn't add anything to the resource so (if it's isn't there already) we may be able to return an error if a resource's depends_on isn't referring to an action sooner than the graph walk.

Copy link
Contributor

@mildwonkey mildwonkey left a comment

Choose a reason for hiding this comment

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

This seems reasonable. I don't need to block this PR, but I think we should see about adding a similar check during configload - depends_on must be static, so we're not worried about evaluation or expansion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1.14-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged no-changelog-needed Add this to your PR if the change does not require a changelog entry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Terraform crash when creating a dependency on action

3 participants