Skip to content

Conversation

KenCox-Hashicorp
Copy link
Contributor

@KenCox-Hashicorp KenCox-Hashicorp commented Aug 1, 2025

Description

Fixes #1061. Note, that was reported in v1.75, the bug is still in head of code v1.88, so this may need backport.

validateAdminRunFilterParams in admin_run.go was using RunCostEstimate, a RunIncludeOpt. It should be using RunCostEstimated, a RunStatus. Adds unit test.

Some of the RunStatus values were not accepted by validateAdminRunFilterParams - this PR assumes that was a mistake. The validate function has been fixed to accept all status values.

Note: Merge either this PR or #1170, which does not update the validate function with the missing RunStatus values.

Testing plan

Code given in the issue should run, since "cost_estimated" is a RunStatus. This now works:

func main() {
     client, err := tfe.NewClient(&tfe.Config{
                Address:           "...",
                RetryServerErrors: true,
                Token:             "...",
        })
        if err != nil {
                log.Fatal("Error %v\n", err)
                os.Exit(1)
        }
        costEstimatedRuns, err := client.Admin.Runs.List(context.Background(), &tfe.AdminRunsListOptions{
                RunStatus: "cost_estimated",
        })
        if err != nil {
                log.Fatal(err)
        }
        log.Printf("Found %v runs in 'cost_estimated' state", len(costEstimatedRuns.Items))
}

Related Links

Output from tests

New unit test does not require TFE instance.

Rollback Plan

Revert the PR.

Changes to Security Controls

None.

@KenCox-Hashicorp KenCox-Hashicorp force-pushed the kencox/1061-fix-costestimated-and-validate branch 2 times, most recently from 843968e to 8b519ff Compare August 1, 2025 20:41
@KenCox-Hashicorp KenCox-Hashicorp marked this pull request as ready for review August 1, 2025 20:43
@KenCox-Hashicorp KenCox-Hashicorp requested a review from a team as a code owner August 1, 2025 20:43
@KenCox-Hashicorp KenCox-Hashicorp force-pushed the kencox/1061-fix-costestimated-and-validate branch 4 times, most recently from 18b012b to 4c61223 Compare August 6, 2025 20:31
validateAdminRunFilterParams in admin_run.go was using RunCostEstimate, a RunIncludeOpt.
It should be using RunCostEstimated, a RunStatus. Adds unit test.

Some of the RunStatus values were not accepted by validateAdminRunFilterParams - this PR assumes that was a mistake.
The validate function has been fixed to accept all status values.
@KenCox-Hashicorp KenCox-Hashicorp force-pushed the kencox/1061-fix-costestimated-and-validate branch from 4c61223 to c1bbd4e Compare August 8, 2025 13:49
Copy link
Collaborator

@brandonc brandonc left a comment

Choose a reason for hiding this comment

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

Thank you for adding these. Before I merge it, it looks like the CHANGELOG entry is still in the wrong place (under 1.89.0) -- Can you move it to underneath # Unrelelased?

@KenCox-Hashicorp KenCox-Hashicorp merged commit c21b599 into main Aug 8, 2025
8 checks passed
@KenCox-Hashicorp KenCox-Hashicorp deleted the kencox/1061-fix-costestimated-and-validate branch August 8, 2025 15:35
Copy link

github-actions bot commented Aug 8, 2025

Reminder to the contributor that merged this PR: if your changes have added important functionality or fixed a relevant bug, open a follow-up PR to update CHANGELOG.md with a note on your changes.

mutahhir pushed a commit that referenced this pull request Sep 2, 2025
* Fix validation of run status; add unit test [issue 1061]

validateAdminRunFilterParams in admin_run.go was using RunCostEstimate, a RunIncludeOpt.
It should be using RunCostEstimated, a RunStatus. Adds unit test.

Some of the RunStatus values were not accepted by validateAdminRunFilterParams - this PR assumes that was a mistake.
The validate function has been fixed to accept all status values.

* fix changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ERROR: invalid value "cost_estimated" for run status
2 participants