support encrypted s3 bucket for plan uploads #1882
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughThe changes introduce support for S3 bucket encryption in the run-digger GitHub action and its associated AWS storage handling. New input parameters for enabling encryption, selecting the encryption type (AES256 or KMS), and specifying a KMS key ID have been added in the action configuration. Corresponding modifications in the AWS storage implementation include new fields for encryption in the storage struct, a new constructor that validates encryption settings, and updates to the file upload process to apply the encryption configuration. Changes
Sequence Diagram(s)sequenceDiagram
participant User as Developer/User
participant Action as GitHub Action (run-digger)
participant CLI as Digger CLI
participant Storage as AWSPlanStorage Constructor
participant S3 as AWS S3
User->>Action: Trigger action execution
Action->>Action: Read S3 encryption inputs & set environment variables
Action->>CLI: Invoke Digger CLI with encryption parameters
CLI->>Storage: Call NewAWSPlanStorage(encryption settings)
Storage-->>CLI: Return configured AWSPlanStorage instance or error
CLI->>S3: Upload plan file with specified encryption settings
Poem
Tip 🌐 Web search-backed reviews and chat
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
libs/storage/aws_plan_storage.go (2)
43-73: Enhance error handling in the constructor.While the validation logic is good, consider improving error messages and handling:
- Validate that
encryptionTypeis not empty when encryption is enabled- Use constants instead of string literals for encryption type comparison
Apply this diff to improve the constructor:
func NewAWSPlanStorage(bucketName string, encryptionEnabled bool, encryptionType string, KMSEncryptionId string) (*PlanStorageAWS, error) { if bucketName == "" { return nil, fmt.Errorf("AWS_S3_BUCKET is not defined") } ctx, client, err := GetAWSStorageClient() if err != nil { return nil, fmt.Errorf("could not retrieve aws storage client") } planStorage := &PlanStorageAWS{ Context: ctx, Client: client, Bucket: bucketName, } if encryptionEnabled { + if encryptionType == "" { + return nil, fmt.Errorf("encryption type must be specified when encryption is enabled") + } planStorage.EncryptionEnabled = true - if encryptionType == "AES256" { + if encryptionType == string(ServerSideEncryptionAes256) { planStorage.EncryptionType = ServerSideEncryptionAes256 - } else if encryptionType == "KMS" { + } else if encryptionType == "KMS" { if KMSEncryptionId == "" { return nil, fmt.Errorf("KMS encryption requested but no KMS key specified") } planStorage.EncryptionType = ServerSideEncryptionAwsKms planStorage.KMSEncryptionId = KMSEncryptionId } else { return nil, fmt.Errorf("unknown encryption type specified for aws plan bucket: %v", encryptionType) } } return planStorage, nil }
97-118: Add error handling for encryption configuration.The encryption configuration in
StorePlanFileshould include error handling for invalid encryption settings.Apply this diff to improve error handling:
func (psa *PlanStorageAWS) StorePlanFile(fileContents []byte, artifactName, fileName string) error { input := &s3.PutObjectInput{ Body: bytes.NewReader(fileContents), Bucket: aws.String(psa.Bucket), Key: aws.String(fileName), } // support for encryption if psa.EncryptionEnabled { + if psa.EncryptionType == "" { + return fmt.Errorf("encryption type not configured") + } input.ServerSideEncryption = types.ServerSideEncryption(psa.EncryptionType) if psa.EncryptionType == ServerSideEncryptionAwsKms { + if psa.KMSEncryptionId == "" { + return fmt.Errorf("KMS key ID not configured for KMS encryption") + } input.SSEKMSKeyId = aws.String(psa.KMSEncryptionId) } } _, err := psa.Client.PutObject(psa.Context, input) if err != nil { log.Printf("Failed to write file to bucket: %v", err) return err } return nil }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
action.yml(3 hunks)libs/storage/aws_plan_storage.go(2 hunks)libs/storage/plan_storage.go(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
🔇 Additional comments (5)
libs/storage/aws_plan_storage.go (2)
27-32: LGTM! Well-defined encryption types.The encryption types are correctly defined using string constants, making the code more maintainable and type-safe.
34-41: LGTM! Appropriate struct fields for encryption.The struct fields are well-organized and properly typed for handling encryption configuration.
libs/storage/plan_storage.go (1)
239-246: LGTM! Good integration of encryption settings.The code correctly retrieves encryption settings from environment variables and passes them to the AWS storage constructor.
action.yml (2)
103-114: LGTM! Well-defined input parameters for S3 encryption.The input parameters are properly defined with clear descriptions and appropriate default values.
382-384: LGTM! Consistent environment variable configuration.The encryption settings are correctly passed as environment variables in both build and run steps.
Also applies to: 422-424
|
Hi, I tried this new feature like this: <upper part of action removed>
- name: digger run
uses: diggerhq/digger@feat/aws-plan-storage-s3-enc
with:
setup-aws: false
setup-terraform: false
setup-opentofu: true
opentofu-version: 1.7.2
no-backend: true
upload-plan-destination: 'aws'
upload-plan-destination-s3-bucket: <removed>
upload-plan-destination-s3-encryption-enabled: true
upload-plan-destination-s3-encryption-type: AES256
# upload-plan-destination-s3-encryption-kms-key-id:
env:
GITHUB_CONTEXT: ${{ toJson(github) }}
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
And it gives me this weird error of not being able to access github: <long long build logs here>
2025-02-07T08:56:40.5582262Z go: downloading github.com/ryanuber/go-glob v1.0.0
2025-02-07T08:56:40.5620749Z go: downloading github.com/golang-jwt/jwt/v5 v5.2.1
2025-02-07T08:56:40.5625100Z go: downloading github.com/hashicorp/go-immutable-radix v1.3.1
2025-02-07T08:58:25.3630326Z 08:58:25 diggerapi.go:334: WARNING: running in 'backendless' mode. Features that require backend will not be available.
2025-02-07T08:58:25.3631175Z 08:58:25 locking.go:252: Using AWS lock provider.
2025-02-07T08:58:25.3631684Z 08:58:25 locking.go:270: Using keyless aws digger_config
2025-02-07T08:58:25.4757616Z 08:58:25 locking.go:282: Successfully connected to AWS account <removed>, user Id: <removed>
2025-02-07T08:58:25.4758676Z 08:58:25 root.go:119: Lock provider has been created successfully
2025-02-07T08:58:25.4759234Z 08:58:25 github.go:29: Using GitHub.
2025-02-07T08:58:25.4759923Z 08:58:25 providers.go:14: WARNING: running in 'backendless' mode. No policies will be supported.
2025-02-07T08:58:25.4774728Z 08:58:25 github.go:72: GitHub context parsed successfully
2025-02-07T08:58:25.4778634Z 08:58:25 github.go:97: Digger digger_config read successfully
2025-02-07T08:58:25.7802434Z 08:58:25 github.go:231: Following projects are impacted by pull request #39
2025-02-07T08:58:25.7803068Z 08:58:25 github.go:233: - default
2025-02-07T08:58:25.7803660Z 08:58:25 github.go:235: GitHub event processed successfully
2025-02-07T08:58:26.0785218Z 08:58:26 github.go:272: GitHub event converted to commands successfully
2025-02-07T08:58:26.0786267Z 08:58:26 github.go:334: Following commands are going to be executed:
2025-02-07T08:58:26.0787106Z project: default: commands: "digger plan",
2025-02-07T08:58:26.0788253Z 08:58:26 digger.go:70: Info: [TF_PLUGIN_CACHE_DIR=/runner/_work/<removed repo>/<removed repo>/cache]
2025-02-07T08:58:26.0789561Z 08:58:26 digger.go:71: Info: [TERRAGRUNT_PROVIDER_CACHE_DIR=/runner/_work/<removed repo>/<removed repo>/cache
2025-02-07T08:58:26.0790614Z 08:58:26 digger.go:188: Running 'digger plan' for project 'default' (workflow: rolechainwf)
2025-02-07T08:58:26.0791289Z 08:58:26 aws.go:113: Using authentication strategy: Default
2025-02-07T08:58:26.2877181Z 08:58:26 github.go:231: error getting pull request (as issue): GET https://api.github.com/repos/<removed org>/<removed repo>/issues/39: 403 Resource not accessible by integration []
2025-02-07T08:58:26.2879535Z 08:58:26 github.go:241: error checking if pull request is issue: error getting pull request (as issue): GET https://api.github.com/repos/<removed org>/<removed repo>/issues/39: 403 Resource not accessible by integration []
2025-02-07T08:58:26.2882206Z 08:58:26 digger.go:100: error while running command digger plan for project default: Failed to set PR status. error checking if pull request is issue: error getting pull request (as issue): GET https://api.github.com/repos/<removed org>/<removed repo>/issues/39: 403 Resource not accessible by integration []
2025-02-07T08:58:26.2884026Z 08:58:26 digger.go:109: Project default command digger plan failed, skipping job
2025-02-07T08:58:26.4622408Z 08:58:26 github.go:231: error getting pull request (as issue): GET https://api.github.com/repos/<removed org>/<removed repo>/issues/39: 403 Resource not accessible by integration []
2025-02-07T08:58:26.4624595Z 08:58:26 github.go:241: error checking if pull request is issue: error getting pull request (as issue): GET https://api.github.com/repos/<removed org>/<removed repo>/issues/39: 403 Resource not accessible by integration []
2025-02-07T08:58:26.4625995Z 08:58:26 usage.go:112: Failed to run commands. %!s(<nil>)
2025-02-07T08:58:26.4655721Z ##[error]Process completed with exit code 8.
2025-02-07T08:58:26.4711191Z Post job cleanup.
2025-02-07T08:58:26.5580979Z Cleanup complete.
2025-02-07T08:58:26.5940909Z Post job cleanup.
2025-02-07T08:58:26.5998280Z Post job cleanup.
2025-02-07T08:58:26.6450294Z ##[warning]Login cleanup failed with Error: Unable to locate executable file: az. Please verify either the file path exists or the file can be found within a directory specified by the PATH environment variable. Also check the file mode to verify the file is executable.. Cleanup will be skipped.
2025-02-07T08:58:26.6530486Z Post job cleanup.
2025-02-07T08:58:26.7636706Z [command]/usr/bin/git version
2025-02-07T08:58:26.7681449Z git version 2.48.1
2025-02-07T08:58:26.7739348Z Copying '/home/runner/.gitconfig' to '/runner/_work/_temp/fe8c9559-e7a3-465d-8dde-cdfd3b3cbee1/.gitconfig'
2025-02-07T08:58:26.7753128Z Temporarily overriding HOME='/runner/_work/_temp/fe8c9559-e7a3-465d-8dde-cdfd3b3cbee1' before making global git config changes
2025-02-07T08:58:26.7754417Z Adding repository directory to the temporary git global config as a safe directory
2025-02-07T08:58:26.7774860Z [command]/usr/bin/git config --global --add safe.directory /runner/_work/<removed repo>/<removed repo>
2025-02-07T08:58:26.7811994Z [command]/usr/bin/git config --local --name-only --get-regexp core\.sshCommand
2025-02-07T08:58:26.7846728Z [command]/usr/bin/git submodule foreach --recursive sh -c "git config --local --name-only --get-regexp 'core\.sshCommand' && git config --local --unset-all 'core.sshCommand' || :"
2025-02-07T08:58:26.8065960Z [command]/usr/bin/git config --local --name-only --get-regexp http\.https\:\/\/github\.com\/\.extraheader
2025-02-07T08:58:26.8085056Z http.https://github.com/.extraheader
2025-02-07T08:58:26.8102969Z [command]/usr/bin/git config --local --unset-all http.https://github.com/.extraheader
2025-02-07T08:58:26.8135340Z [command]/usr/bin/git submodule foreach --recursive sh -c "git config --local --name-only --get-regexp 'http\.https\:\/\/github\.com\/\.extraheader' && git config --local --unset-all 'http.https://github.com/.extraheader' || :"
2025-02-07T08:58:26.8440533Z A job completed hook has been configured by the self-hosted runner administrator
2025-02-07T08:58:26.8458692Z ##[group]Run '/etc/arc/hooks/job-completed.sh'
2025-02-07T08:58:26.8466630Z shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
2025-02-07T08:58:26.8467093Z ##[endgroup]
2025-02-07T08:58:26.8513279Z �[0;37m2025-02-07 08:58:26.850 DEBUG --- Running ARC Job Completed Hooks�[0m
2025-02-07T08:58:26.8527889Z �[0;37m2025-02-07 08:58:26.852 DEBUG --- Running hook: /etc/arc/hooks/job-completed.d/update-status�[0m
2025-02-07T08:58:26.8612104Z Cleaning up orphan processes |
|
@joerg the issue you faced is related to this issue I believe, you need to ensure you have issues:write in the workflow permissions |
UtpalJayNadiger
approved these changes
Feb 11, 2025
|
I've tested a plan-apply cycle for S3 AES256 and KMS separately, it seems to work fine, for now I am happy merging this PR and doing a release to unblock folks 
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add support for S3 bucket encryption when performing plan uploads, addressing #1783
The additional arguments introduced are:
upload-plan-destination-s3-encryption-enabled: true/false to specify if you want to enable s3 bucket encryption support
upload-plan-destination-s3-encryption-type: AES256 is default, allowed values is AES256 or KMS
upload-plan-destination-s3-encryption-kms-key-id: if encryption is KMS then you need to specify KMS ID or ARN of KMS bucket
Summary by CodeRabbit