[Cloud Security] Add aws prefix to CSPM vars
#14923
Conversation
seanrathier
added
enhancement
There was a problem hiding this comment.
Pull Request Overview
This PR adds an aws prefix to CSPM (Cloud Security Posture Management) variable names to enable shared input variables between CSPM and Cloud Asset Inventory components for CloudSetup integration.
- Prefixed AWS credential-related variables with
aws.namespace (e.g.,access_key_id→aws.access_key_id) - Maintained backward compatibility by supporting both old and new variable names in templates
- Updated package version to 4.0.0-preview01 to reflect the significant change
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/cloud_security_posture/manifest.yml | Updated package version to 4.0.0-preview01 |
| packages/cloud_security_posture/data_stream/findings/manifest.yml | Added aws prefix to credential variable names in stream configurations |
| packages/cloud_security_posture/data_stream/findings/agent/stream/aws.yml.hbs | Added support for both old and new prefixed variable names in Handlebars template |
| packages/cloud_security_posture/changelog.yml | Added changelog entry documenting the AWS variable prefix enhancement |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
| @@ -23,21 +23,39 @@ config: | |||
| {{#if access_key_id}} | |||
There was a problem hiding this comment.
The template now contains duplicate conditional blocks for both old and new variable names (e.g., access_key_id and aws.access_key_id). This duplication makes the template harder to maintain. Consider consolidating these into a single conditional or adding a comment explaining the backward compatibility strategy and deprecation timeline.
Copilot uses AI. Check for mistakes.
🚀 Benchmarks reportTo see the full report comment with |
💚 Build Succeeded
cc @seanrathier |
|
andrewkroh
added
the
Integration:cloud_security_posture
| @@ -15,6 +15,11 @@ | |||
| # 1.4.x - 8.9.x | |||
| # 1.3.x - 8.8.x | |||
| # 1.2.x - 8.7.x | |||
| - version: "4.0.0-preview01" | |||
There was a problem hiding this comment.
what's the reasoning behind major version upgrade?
So far, we kept it in sync with major stack versions
There was a problem hiding this comment.
We are at version 3. I perceived this to be a breaking change, warranting a major version change.
| @@ -23,21 +23,39 @@ config: | |||
| {{#if access_key_id}} | |||
| access_key_id: {{access_key_id}} | |||
| {{/if}} | |||
| {{#if aws.access_key_id}} | |||
There was a problem hiding this comment.
what happens if both access_key_id and aws.access_key_id are available?
I believe only one should be used to resolve source of truth
access_key_id is deprecated and its content is migrated to aws.access_key_id. then I'd use aws.access_key_id when available, and when not I'll check for access_key_id for compatibility
There was a problem hiding this comment.
I was doing that in the aws.hbs file, the change checks for both values with and without the aws prefix. Or so I thought that was the solution you were referring to in DMs.
However, this change does not address showing the Fleet extension in Kibana for older versions of the integration; therefore, I am going to close this PR and have a CloudSetup mapping
| - version: "4.0.0-preview01" | ||
| changes: | ||
| - description: Changed AWS input vars with aws. prefix | ||
| type: enhancement |
There was a problem hiding this comment.
I noticed the GH PR is labeled with breaking change but that's not in agreement with the changelog type. Assuming it's a breaking change that affects end users then I would expect to have a changes item with type: breaking-change explaining the impact and how users can handle the breaking change when they upgrade.
There was a problem hiding this comment.
@andrewkroh, I was pretty sure that this would be a breaking eventually, however I managed to find a different path in Kibana to fix this issue so I will be closing this.
I will add the correct type next time.
|
I am closing this PR because there is a path in Kibana that is less risky and to support older versions of the CSPM integration. |
Proposed commit message
Add
awsprefix to CSPM varsThis change is needed so we can use the same input variables for CSPM and Cloud Asset Inventory in order to make integration with the CloudSetup component.
Checklist
changelog.ymlfile.Related issues