Skip to content

feat(gemini): Provisions a temporary cloud outpost (AWS EC2 instance) that includes a self-destruct sequence reminder.#5121

Open
polsala wants to merge 1 commit into
mainfrom
ai/gemini-20260706-1523
Open

feat(gemini): Provisions a temporary cloud outpost (AWS EC2 instance) that includes a self-destruct sequence reminder.#5121
polsala wants to merge 1 commit into
mainfrom
ai/gemini-20260706-1523

Conversation

@polsala

@polsala polsala commented Jul 6, 2026

Copy link
Copy Markdown
Owner

Implementation Summary

  • Utility: nightly-temporal-outpost-prov
  • Provider: gemini
  • Location: terraform-modules/nightly-nightly-temporal-outpost-pro
  • Files Created: 5
  • Description: Provisions a temporary cloud outpost (AWS EC2 instance) that includes a self-destruct sequence reminder.

Rationale

  • Automated proposal from the Gemini generator delivering a fresh community utility.
  • This utility was generated using the gemini AI provider.

Why safe to merge

  • Utility is isolated to terraform-modules/nightly-nightly-temporal-outpost-pro.
  • README + tests ship together (see folder contents).
  • No secrets or credentials touched.
  • All changes are additive and self-contained.

Test Plan

  • Follow the instructions in the generated README at terraform-modules/nightly-nightly-temporal-outpost-pro/README.md
  • Run tests located in terraform-modules/nightly-nightly-temporal-outpost-pro/tests/

Links

  • Generated docs and examples committed alongside this change.

Mock Justification

  • Not applicable; generator did not introduce new mocks.

… that includes a self-destruct sequence reminder.
@polsala

polsala commented Jul 6, 2026

Copy link
Copy Markdown
Owner Author

🤖 Review by GROQ Agent

✅ What's solid

  • Clear module purpose – the Terraform files are well‑organized (provider, security group, EC2 instance, outputs, variables) and the intent (a short‑lived “temporal outpost”) is obvious.
  • Self‑destruct reminder – the destroy_command and self_destruct_reminder outputs give users a handy, ready‑to‑copy command, which matches the whimsical theme of the utility.
  • Automated plan test – the test_plan.sh script runs a dry‑run (terraform plan) with deterministic variables and checks for the expected resources and static outputs. This gives a quick sanity check before any real apply.
  • No secrets in code – the module does not embed any credentials or hard‑coded keys, which keeps the PR safe from accidental leakage.

🧪 Tests

Observation Recommendation
The test only validates that the aws_instance and aws_security_group appear in the plan and that the static outputs are present. Add a terraform validate step to catch syntax or provider‑configuration errors early. Example snippet for the script:

bash\nterraform validate\n
The script does not verify that the generated outputs have the correct type (e.g., that public_ip is marked “known after apply”). Assert output meta‑information to ensure the plan reflects the expected “known after apply” state, e.g.:

```bash\nif ! echo "$PLAN_OUTPUT"
No linting/format check is performed. Add terraform fmt -check to enforce canonical formatting.
The test runs terraform init -backend=false but the module lacks a terraform { required_providers { … } } block, which can cause a warning or failure on newer Terraform versions. Add a minimal terraform block (see Security section) and update the test to run terraform init -backend=false -upgrade to ensure provider constraints are respected.
The test script is placed under tests/ but the README points to ./tests/test_plan.sh from the module root. This works, but the script changes directory to src/. Document the directory change in the README or make the script robust by using SCRIPT_DIR=$(dirname "$0") and cd "$SCRIPT_DIR/../src" to avoid path confusion.

🔒 Security

  • Security group is wide open – both SSH (22) and HTTP (80) are allowed from 0.0.0.0/0. This is acceptable for a demo, but it should be highlighted as a dangerous default and ideally gated behind a variable (e.g., allowed_cidr_blocks).
    variable "allowed_cidr_blocks" {
      type    = list(string)
      default = ["0.0.0.0/0"]
    }
    # then use var.allowed_cidr_blocks in the ingress rules
  • Placeholder AMI – the default ami-0abcdef1234567890 is not a real image. Terraform will still accept it in a plan, but an apply will fail. Add a validation rule that forces the user to supply a non‑empty, region‑compatible AMI, or provide a data source that looks up the latest Amazon Linux 2 AMI for the selected region.
    data "aws_ami" "default" {
      most_recent = true
      owners      = ["amazon"]
      filter {
        name   = "name"
        values = ["amzn2-ami-hvm-*-x86_64-gp2"]
      }
    }
    
    variable "ami" {
      description = "AMI ID for the EC2 instance."
      type        = string
      default     = null
    }
    
    locals {
      ami_id = var.ami != null ? var.ami : data.aws_ami.default.id
    }
  • Missing provider version constraints – without a required_providers block, the module may silently upgrade to a breaking provider version. Add a terraform { required_providers { aws = { source = "hashicorp/aws", version = "~> 5.0" } } } block.
  • No IAM role or instance profile – the EC2 instance runs with the default AWS credentials of the user who applies the module. If the outpost ever needs to perform actions (e.g., write logs), consider attaching a minimal IAM role. Even if not needed now, documenting the omission helps future maintainers.
  • Potential for orphaned resources – the “self‑destruct” is purely manual. Adding a time_sleep resource with a null_resource that triggers a terraform destroy after the configured minutes could automate cleanup (or at least warn).

🧩 Docs / Developer Experience

  • Path inconsistencies – the README references terraform-modules/nightly-temporal-outpost-prov/src while the actual directory is terraform-modules/nightly-nightly-temporal-outpost-pro. Align the documentation with the real path to avoid confusion.
  • Usage steps could be streamlined – combine the cd and terraform init commands into a single helper script (e.g., ./run.sh) that sets the working directory automatically.
  • Variable documentation – the README lists the variables, but it does not show the default values or the type of each variable. Adding a table with Name, Type, Default, Description would make it easier for users to override them.
  • Self‑destruct reminder wording – the output string includes a hard‑coded apostrophe (') which can break shell quoting in some contexts. Consider escaping or using double quotes consistently.
  • Testing instructions – the README says “navigate to the utility's root directory and execute the test script”, but the script itself cds into src. Clarify that the command should be run from the module root (./tests/test_plan.sh).
  • Formatting – run terraform fmt locally before committing; the current files appear correctly formatted, but adding a CI check will enforce consistency.

🧱 Mocks / Fakes

  • Provider “mock” via -backend=false – this is a reasonable approach for a dry‑run, but the AWS provider still attempts to validate certain arguments (e.g., region). Adding a provider "aws" { skip_credentials_validation = true skip_metadata_api_check = true } block for the test environment can make the plan completely offline and faster.
    provider "aws" {
      region                      = var.region
      skip_credentials_validation = true
      skip_metadata_api_check     = true
    }
  • No explicit mock resources – the module does not use any data sources that would require network calls, which keeps the plan fast. If future extensions need to query VPC IDs or subnets, consider using the terraform-provider-mock or local data sources with static values for CI.
  • Test script could use environment variables – instead of passing -var flags, you can set TF_VAR_region=us-east-1 etc. This mirrors how many CI pipelines inject variables and reduces the risk of a typo in the flag list.

Summary of quick wins

  1. Add a terraform { required_providers { … } } block with a pinned AWS provider version.
  2. Restrict the security group via a configurable CIDR variable and document the risk of the default open rule.
  3. Replace the placeholder AMI with a data source lookup or enforce a non‑null validation.
  4. Fix README path mismatches and improve the variable table for better DX.
  5. Enhance the test script with terraform validate, terraform fmt -check, and optional provider‑skip flags.

Implementing these suggestions will tighten security, improve reliability of the CI test, and make the module easier for downstream users to adopt. Happy Terraforming!

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.

1 participant