Add toggle for provisioning default permission sets

[petabook]

Summary

This PR introduces a new option that lets users control whether the default permission sets are automatically provisioned. The change is primarily cosmetic, intended to accommodate different naming conventions for these standard permission sets, rather than to alter core functionality.

Details

• A new variable, provision_sensible_permission_sets, has been added.
  Default: true - preserves the existing behavior.
• When set to false, the following permission sets will not be provisioned:
  • AdministratorAccess
  • BillingAdministratorAccess
  • BillingReadOnlyAccess
  • DNSRecordAdministratorAccess
  • PowerUserAccess
  • ReadOnlyAccess

Summary by CodeRabbit

• New Features
  • Added configuration option to control provisioning of default AWS SSO permission sets. Enabled by default to maintain backward compatibility while allowing users to customize permission set configurations.

[coderabbitai]

Walkthrough

Two files modified: a new boolean variable controls whether default AWS SSO permission sets are provisioned. The permission_sets configuration in main.tf now conditionally includes sensible defaults based on this variable, using a ternary operator to either concatenate the defaults or return an empty list.

Changes

Cohort / File(s)	Summary
Conditional permission set provisioning src/main.tf	Refactors permission_sets input to conditionally augment defaults. Sensible permission sets (administrator, billing admin, billing read-only, DNS admin, poweruser, read-only) are now included only when the new flag is enabled, wrapped in a ternary that yields either concatenated locals or an empty list.
New configuration variable src/variables.tf	Adds provision_sensible_permission_sets boolean variable with default true to enable/disable provisioning of sensible AWS SSO permission set defaults.

Sequence Diagram

sequenceDiagram
    actor User
    participant Config as Terraform Config
    participant Module as cloudposse/sso/aws Module

    User->>Config: Set provision_sensible_permission_sets = true/false
    activate Config
    
    alt provision_sensible_permission_sets == true
        Config->>Config: Include sensible permission sets<br/>(admin, billing, DNS, poweruser, read-only)
    else provision_sensible_permission_sets == false
        Config->>Config: Skip sensible permission sets
    end
    
    Config->>Config: Concatenate all permission set lists:<br/>- overridable_additional<br/>- identity_access<br/>- terraform_update_access<br/>- sensible (conditional)
    deactivate Config
    
    Config->>Module: Provide final permission_sets list
    Module->>Module: Provision SSO permission sets

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

The changes are straightforward: a new boolean variable and conditional wrapping logic. Minimal scope affecting only two files with clear, consistent patterns. No complex domain logic or cross-cutting concerns.

Suggested labels

minor, needs-test

Suggested reviewers

• goruha

Poem

🐰 Permission sets now bend to will,
A flag to choose which perms to fill,
Sensible defaults stand at the gate,
True provisioned, or left to fate,
Terraform's dance, so clean and bright! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The PR title "Add toggle for provisioning default permission sets" directly and accurately describes the primary change in the pull request. The changeset introduces a new boolean variable provision_sensible_permission_sets that conditionally controls whether six default permission sets (administrator, billing admin, billing read-only, DNS admin, poweruser, read-only) are provisioned. The title is concise, clear, and uses specific terminology ("toggle," "provisioning," "default permission sets") that would allow a teammate scanning the history to immediately understand the core change without confusion or ambiguity.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ff345bc and 11c91ce.

📒 Files selected for processing (2)

• src/main.tf (1 hunks)
• src/variables.tf (1 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

src/{main,variables,outputs,providers,versions,context}.tf

📄 CodeRabbit inference engine (AGENTS.md)

Keep the Terraform component source of truth in src with these files present: main.tf, variables.tf, outputs.tf, providers.tf, versions.tf, context.tf

Files:

• src/variables.tf
• src/main.tf

src/**/*.tf

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.tf: Use 2-space indentation for all Terraform code
In Terraform, prefer lower_snake_case for variables and locals; keep resource/data source names descriptive and aligned with Cloud Posse null-label patterns
Run terraform fmt and adhere to formatting (do not commit formatting violations)
Adhere to TFLint rules defined for the project (do not commit lint violations)

Files:

• src/variables.tf
• src/main.tf

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Summary

🔇 Additional comments (2)

src/variables.tf (1)

80-84: Well-structured variable addition.

The new variable correctly implements the feature toggle with proper naming, clear description, and a sensible default that preserves existing behavior.

src/main.tf (1)

80-92: Code is correct—remove invalid refactor suggestion.

All six sensible permission set locals are properly defined across policy files as list-type locals (administrator_access_permission_set = [{...}], etc.). The current implementation using concat() is correct for flattening multiple lists into a single result. The suggested refactor to bracket notation would create a list of lists instead and should not be applied.

The conditional logic correctly gates these permission sets via the var.provision_sensible_permission_sets toggle, maintaining backward compatibility.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}