TF-28651 deprecate endpoints#1205
Conversation
|
|
|
Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement Learn more about why HashiCorp requires a CLA and what the CLA includes Have you signed the CLA already but the status is still pending? Recheck it. |
ctrombley
left a comment
ctrombley
left a comment
There was a problem hiding this comment.
Hey @sahar-azizighannad, this looks good so far! I left a question about the update to TestStackConverged.
| ## Enhancements | ||
|
|
||
| * Updates endpoint for deleting stack from `POST` to `DELETE` and removing `/delete` from endpoint by | ||
| * Updates endpoint for force deleting stack from `POST` to `DELETE` and removing `/force-delete` from endpoint and adding `?force=true` by | ||
|
|
There was a problem hiding this comment.
These enhancements aren't customer-impacting, correct? If not I don't think they need to show in the changelog.
There was a problem hiding this comment.
Shouldn't user be able to delete the stack? The post wont work anymore.
| for _, deployment := range deployments { | ||
| pollStackDeploymentStatus(t, ctx, client, stack.ID, deployment, "paused") | ||
| } | ||
|
|
There was a problem hiding this comment.
Why was this removed? Is this only needed for testing beta code? Seems like we would still need the ability to poll stack deployment status for this test even after the refactor.
There was a problem hiding this comment.
We do not support this endpoint on GA. Unless I move it back instead of deprecating which I can.
There was a problem hiding this comment.
OK I think I get it, we are still able to test if the stack converged even without checking the stack deployment statuses. Makes sense.
There was a problem hiding this comment.
Pull Request Overview
This PR removes several deprecated endpoints from the go-tfe client library as part of the Stacks GA preparation. The changes involve removing client fields, implementation files, and associated integration tests for stack plans, stack plan operations, stack sources, and stack deployments.
Key changes include:
- Removal of deprecated client interfaces and implementations for stack plans, operations, sources, and deployments
- Update of stack deletion endpoints from POST to DELETE with simplified URLs
- Cleanup of integration tests that relied on deprecated functionality
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tfe.go | Removes deprecated client fields and initializations for stack plans, operations, sources, and deployments |
| stack_source.go | Complete removal of StackSources interface and implementation |
| stack_plan.go | Complete removal of StackPlans interface and implementation |
| stack_plan_operation.go | Complete removal of StackPlanOperations interface and implementation |
| stack_deployments.go | Complete removal of StackDeployments interface and implementation |
| stack.go | Updates Delete and ForceDelete methods to use proper REST verbs and simplified endpoints |
| stack_*_integration_test.go | Removes integration tests for deprecated endpoints |
| stack_integration_test.go | Removes test code that used deprecated StackDeployments interface |
| CHANGELOG.md | Documents the deprecations and endpoint changes |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Chris Trombley <ctrombley@gmail.com>
| client *Client | ||
| } | ||
|
|
||
| func (s stackDeployments) Read(ctx context.Context, stackID, deploymentName string) (*StackDeployment, error) { |
There was a problem hiding this comment.
Should we remove the StackDeployment struct definition from stacks.go? Seems like it is not being used anymore
CHANGELOG.md
Outdated
| * Updates endpoint for deleting stack from `POST` to `DELETE` and removing `/delete` from endpoint by @sahar-azizighannad [#1205](https://github.com/hashicorp/go-tfe/pull/1205) | ||
| * Updates endpoint for force deleting stack from `POST` to `DELETE` and removing `/force-delete` from endpoint and adding `?force=true` by @sahar-azizighannad [#1205](https://github.com/hashicorp/go-tfe/pull/1205) |
There was a problem hiding this comment.
I think what Chris meant in his comment here didn't pertain to the change being made but rather it being called out in the CHANGELOG. Since updating the routes isn't a user facing change, we can omit these entries.
|
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. |
Description
We will deprecate few endpoints for stacks GA.
Testing plan
Integration test have been also removed.
External links
Output from tests
Changes to Security Controls