Skip to content

fix: merge cleanup policies#24

Merged
FabrizioCafolla merged 2 commits into
mainfrom
fix/merge-cleanup-policies
Oct 28, 2025
Merged

fix: merge cleanup policies#24
FabrizioCafolla merged 2 commits into
mainfrom
fix/merge-cleanup-policies

Conversation

@FabrizioCafolla

@FabrizioCafolla FabrizioCafolla commented Oct 28, 2025

Copy link
Copy Markdown
Member

PR Type

Bug fix, Enhancement


Description

  • Simplified cleanup policies merge logic by removing tomap() wrapper

  • Added output for final repository configurations after defaults merge


Diagram Walkthrough

flowchart LR
  A["Repository Config"] --> B["Merge Cleanup Policies"]
  B --> C["Final Repository Config"]
  C --> D["New Output"]
Loading

File Walkthrough

Relevant files
Bug fix
main.tf
Simplify cleanup policies merge logic                                       

main.tf

  • Simplified cleanup policies merge by removing unnecessary tomap({})
    conversion
  • Changed empty map fallback from tomap({}) to {} for cleaner syntax
+1/-4     
Enhancement
outputs.tf
Add repositories final configuration output                           

outputs.tf

  • Added new output repositories_configurations exposing final repository
    configurations
  • Output includes merged cleanup policies and defaults for
    debugging/validation
+5/-0     

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

This PR simplifies the merge logic for cleanup policies in the Artifact Registry module by consolidating a multi-line merge operation into a single line, and adds a new output to expose the final repository configurations after merging with defaults.

  • Simplified cleanup policies merge logic by removing unnecessary tomap({}) call
  • Added new output repositories_configurations to expose final repository configurations

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
main.tf Consolidated cleanup policies merge logic into single line
outputs.tf Added output for final repository configurations

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread outputs.tf Outdated
@sparkfabrik-ai-bot

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 Security concerns

Sensitive information exposure:
The new output repositories_configurations at line 21-24 in outputs.tf exposes the entire local.repositories_final configuration. This could potentially leak sensitive data such as repository credentials, access tokens, or internal configuration details through Terraform state files and output logs. Consider adding sensitive = true to this output block or limiting what information is exposed.

⚡ Recommended focus areas for review

Sensitive Data Exposure

The new output repositories_configurations exposes the complete final repository configuration, which may contain sensitive information such as credentials, secrets, or internal configuration details that should not be exposed in Terraform outputs. Consider whether this output is necessary or if it should be marked as sensitive.

output "repositories_configurations" {
  value = local.repositories_final
  description = "The final configuration of the Artifact Registry repositories after merging with defaults."
}

@sparkfabrik-ai-bot

Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Security
Mark output as sensitive

The output exposes the entire repositories_final local variable, which may contain
sensitive information like cleanup policies and repository configurations. Consider
marking this output as sensitive to prevent accidental exposure in logs or console
output.

outputs.tf [21-24]

 output "repositories_configurations" {
   value = local.repositories_final
   description = "The final configuration of the Artifact Registry repositories after merging with defaults."
+  sensitive = true
 }
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that repositories_final may contain sensitive configuration data. Marking the output as sensitive = true is a valid security practice to prevent accidental exposure in logs, though the actual sensitivity depends on the repository configurations being managed.

Medium

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread main.tf
@FabrizioCafolla FabrizioCafolla merged commit 6b570ed into main Oct 28, 2025
1 check passed
@FabrizioCafolla FabrizioCafolla deleted the fix/merge-cleanup-policies branch October 28, 2025 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants