Skip to content

remove terraform tests and package #13154

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
alexrashed merged 2 commits into from
Sep 18, 2025
Merged

remove terraform tests and package #13154

alexrashed merged 2 commits into from
Sep 18, 2025
+0 −313

Conversation

@alexrashed
Copy link
Member

@alexrashed alexrashed commented Sep 17, 2025 •
edited
Loading

Motivation

In the past we have been executing terraform tests directly in our test suit against LocalStack.
However, they were quite flaky and were skipped 2 years ago with #9080.
In the meantime this code had to be maintained, and the terraform package can cause false positives when looking at the security of the Docker image.
This is why this PR removes both, the skipped tests and the package installer (which isn't used anywhere else than in tests here in LocalStack).

Changes

  • Removes skipped test module tests/aws/test_terraform.py.
  • Removes localstack-core/localstack/packages/terraform.py.

TODO

@alexrashed alexrashed added this to the 4.9 milestone Sep 17, 2025
@alexrashed alexrashed added area: integration/terraform Issues related to HashiCorp Terraform semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases docs: skip Pull request does not require documentation changes labels Sep 17, 2025
@alexrashed alexrashed mentioned this pull request Sep 17, 2025
Merged
@github-actions
Copy link

github-actions bot commented Sep 17, 2025 •
edited
Loading

Test Results - Preflight, Unit

22 172 tests  +12   20 434 ✅ +12   6m 31s ⏱️ ±0s
     1 suites ± 0    1 738 💤 ± 0 
     1 files   ± 0        0 ❌ ± 0 

Results for commit 93d99a3. ± Comparison against base commit c8f43c9.

♻️ This comment has been updated with latest results.

@alexrashed alexrashed force-pushed the remove-terraform-package branch from 214137f to 632d631 Compare September 17, 2025 12:25
@github-actions
Copy link

github-actions bot commented Sep 17, 2025 •
edited
Loading

Test Results (amd64) - Acceptance

7 tests  ±0   5 ✅ ±0   3m 20s ⏱️ +5s
1 suites ±0   2 💤 ±0 
1 files   ±0   0 ❌ ±0 

Results for commit 93d99a3. ± Comparison against base commit c8f43c9.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Sep 17, 2025 •
edited
Loading

Test Results (amd64) - Integration, Bootstrap

    5 files      5 suites   2h 38m 57s ⏱️
5 037 tests 4 556 ✅ 481 💤 0 ❌
5 043 runs  4 556 ✅ 487 💤 0 ❌

Results for commit 93d99a3.

♻️ This comment has been updated with latest results.

@localstack localstack deleted a comment from localstack-bot Sep 17, 2025
@github-actions
Copy link

github-actions bot commented Sep 17, 2025 •
edited
Loading

LocalStack Community integration with Pro

    2 files  ±0      2 suites  ±0   1h 58m 46s ⏱️ +40s
4 663 tests  - 9  4 342 ✅ +1  321 💤  - 10  0 ❌ ±0 
4 665 runs   - 9  4 342 ✅ +1  323 💤  - 10  0 ❌ ±0 

Results for commit 93d99a3. ± Comparison against base commit c8f43c9.

This pull request removes 10 and adds 1 tests. Note that renamed tests count towards both. 
tests.aws.test_terraform.TestTerraform ‑ test_acm
tests.aws.test_terraform.TestTerraform ‑ test_apigateway
tests.aws.test_terraform.TestTerraform ‑ test_apigateway_escaped_policy
tests.aws.test_terraform.TestTerraform ‑ test_bucket_exists
tests.aws.test_terraform.TestTerraform ‑ test_dynamodb
tests.aws.test_terraform.TestTerraform ‑ test_event_source_mapping
tests.aws.test_terraform.TestTerraform ‑ test_lambda
tests.aws.test_terraform.TestTerraform ‑ test_route53
tests.aws.test_terraform.TestTerraform ‑ test_security_groups
tests.aws.test_terraform.TestTerraform ‑ test_sqs

tests.aws.services.cloudformation.test_change_sets ‑ test_list_change_sets

♻️ This comment has been updated with latest results.

@alexrashed alexrashed marked this pull request as ready for review September 17, 2025 17:10
bentsku
bentsku approved these changes Sep 17, 2025
View reviewed changes
Copy link
Contributor

@bentsku bentsku left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for cleaning up! 🧹

I'll duplicate my comment I've made on the other PRs just in case 😄

Do we know if any customers were relying on the package being available in LPM? I don't know how much it is used.

We also discussed with @localstack/falcon as part of our IaC Usability improvements that we might want to have Terraform being more prominent in our test suite, at least with some smoke tests.

But I believe we should do it differently that how it was done, especially the installation, and they maybe should be part of a nightly pipeline. Just mentioning it for posterity 😄

In any case, it seems like this is causing more trouble for no gain at the moment, so let's clean up! 🚀

@bentsku
Copy link
Contributor

bentsku commented Sep 17, 2025 •
edited
Loading

@alexrashed oops sorry, while reviewing other PR, I spotted the following: we setup Terraform in our CI:

@alexrashed
Copy link
Member Author

alexrashed commented Sep 18, 2025

Do we know if any customers were relying on the package being available in LPM?

We can't really know exactly how much it is used though because we don't have analytics on it. However, LPM is an internal API and terraform is not installed by default and this should only be the case for init scripts (where we can directly refer to the extension).

@alexrashed alexrashed merged commit 4012f01 into main Sep 18, 2025
44 checks passed
@alexrashed alexrashed deleted the remove-terraform-package branch September 18, 2025 10:56
@alexrashed alexrashed added the notes: needed Pull request should be mentioned in the release notes label Sep 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bentsku bentsku bentsku approved these changes

@dominikschubert dominikschubert Awaiting requested review from dominikschubert dominikschubert is a code owner

@dfangl dfangl Awaiting requested review from dfangl dfangl is a code owner

@silv-io silv-io Awaiting requested review from silv-io silv-io is a code owner

@k-a-il k-a-il Awaiting requested review from k-a-il k-a-il is a code owner

Assignees

@bentsku bentsku

Labels

area: integration/terraform Issues related to HashiCorp Terraform docs: skip Pull request does not require documentation changes notes: needed Pull request should be mentioned in the release notes semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases

Projects

None yet

Milestone

4.9

Development

Successfully merging this pull request may close these issues.

3 participants

@alexrashed @bentsku