Skip to content
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.

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

Associate kms key #1567

Open
wants to merge 2 commits into
base: main-opsathon
Choose a base branch
from
Open

Associate kms key #1567

wants to merge 2 commits into from

Conversation

Paramadon
Copy link
Contributor

@Paramadon Paramadon commented Feb 25, 2025

Description of the issue

Currently customer can not pass in kms key id in for log group to assoicate their log groups to there kms key. This pr allow customer to pass in kms_key_id for there log groups.

Example agent config:

{
  "logs": {
    "logs_collected": {
      "files": {
        "collect_list": [
          {
            "file_path": "/var/log/myapp.log",
            "log_group_name": "my-app-logs",
            "log_stream_name": "{instance_id}",
            "encoding": "utf-8",
            "kms_key_id": "arn:aws:kms:us-west-2:12345:key/YOUR-KMS-KEY-ID"
          }
        ]
      }
    }
  }
}

Description of changes

License

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Tests

Ran test to check if there is not kms key or if kms key is wrong or if api request fails and make sure we fails safely and don't panic. Also tested and log group creation works (log groups have kms key id assoicated with them)

Screenshot 2025-02-26 at 12 37 46 PM

Requirements

Before commit the code, please do the following steps.

  1. Run make fmt and make fmt-sh
  2. Run make lint

@Paramadon Paramadon requested a review from a team as a code owner February 25, 2025 18:45
@Paramadon Paramadon changed the base branch from main to main-opsathon February 26, 2025 16:52
@@ -1,7 +1,7 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: MIT

package collect_list
package collectlist
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Needed to change package name due to lint issues

TravisStark
TravisStark previously approved these changes Feb 26, 2025
@@ -77,7 +77,7 @@ func TestCreateDestination(t *testing.T) {
pusherStopChan: make(chan struct{}),
cwDests: make(map[pusher.Target]*cwDest),
}
dest := c.CreateDest(testCase.cfgLogGroup, testCase.cfgLogStream, testCase.cfgLogRetention, testCase.cfgLogClass, testCase.cfgTailerSrc).(*cwDest)
dest := c.CreateDest(testCase.cfgLogGroup, testCase.cfgLogStream, "", testCase.cfgLogRetention, testCase.cfgLogClass, testCase.cfgTailerSrc).(*cwDest)
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a unit test when kms id is not empty and validate the dest for the key

Copy link
Contributor

github-actions bot commented Mar 6, 2025

This PR was marked stale due to lack of activity.

@github-actions github-actions bot added the Stale label Mar 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants