-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[CloudSecurity][Fleet] Add CloudFormation install method to CSPM #159994
Conversation
Pinging @elastic/kibana-cloud-security-posture (Team:Cloud Security) |
Pinging @elastic/fleet (Team:Fleet) |
x-pack/plugins/fleet/public/services/get_cloud_formation_template_url_from_package_policy.ts
Show resolved
Hide resolved
x-pack/plugins/fleet/public/components/cloud_formation_guide.tsx
Outdated
Show resolved
Hide resolved
...ate_package_policy_page/single_page_layout/components/post_install_cloud_formation_modal.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, added a few suggestions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
left some suggestions mainly around simplifying readability
const checkCurrentTemplate = newPolicy?.inputs?.find((i: any) => i.type === CLOUDBEAT_AWS) | ||
?.config?.cloud_formation_template_url?.value; | ||
|
||
if (setupFormat !== 'cloudFormation') { | ||
if (checkCurrentTemplate !== null) { | ||
updateCloudFormationPolicyTemplate(newPolicy, updatePolicy, null); | ||
} | ||
return; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can return undefined
const checkCurrentTemplate = newPolicy?.inputs?.find((i: any) => i.type === CLOUDBEAT_AWS)
?.config?.cloud_formation_template_url?.value;
so this:
if (checkCurrentTemplate !== null) {
updateCloudFormationPolicyTemplate(newPolicy, updatePolicy, null);
}
essentially checks for undefined !== null
which will result in true, and updateCloudFormationPolicyTemplate
will get called.
], | ||
}, | ||
], | ||
} as PackageInfo; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might be better to type the return, with as
we might make mistakes that won't get noticed.
x-pack/plugins/cloud_security_posture/public/common/api/use_package_policy_list.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/cloud_security_posture/public/components/fleet_extensions/utils.ts
Show resolved
Hide resolved
This one is important, happened to me and it's very confusing |
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
Page load bundle
Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
left some comments, i pulled the or and the overall flow seemed ok, i think its safe to merge so we can start testing it.
also, I don't think it's considered a blocker but I didn't have a fleet on my local env so I installed one locally. the installation was successful and I could follow it up by adding an agent to my integration (directly from fleet installation flyout). but when i clicked on add agent, I got the wrong form:
After that, closing and reopening the flyout did open the correct form with the cloudformation option. so it seems it's only a problem when coming from the fleet flyout, and we can push a separate fix for it.
cc: @kfirpeled @nick-alayil
import { SetupFormat, useAwsCredentialsForm } from './hooks'; | ||
|
||
interface AWSSetupInfoContentProps { | ||
integrationLink: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we rename this? the current name suggest its a link to an integration
input: Extract<NewPackagePolicyPostureInput, { type: 'cloudbeat/cis_aws' }>; | ||
updatePolicy(updatedPolicy: NewPackagePolicy): void; | ||
packageInfo: PackageInfo; | ||
onChange: any; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing type
@@ -15,7 +15,7 @@ describe('getPosturePolicy', () => { | |||
['cloudbeat/cis_k8s', getMockPolicyK8s, null], | |||
] as const) { | |||
it(`updates package policy with hidden vars for ${name}`, () => { | |||
const inputVars = getPostureInputHiddenVars(name); | |||
const inputVars = getPostureInputHiddenVars(name, {} as any); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as PackageInfo
is preferable
const template: string | undefined = newPolicy?.inputs?.find((i) => i.type === CLOUDBEAT_AWS) | ||
?.config?.cloud_formation_template_url?.value; | ||
|
||
return template || undefined; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if template can either be string or undefined, || undefined
is redundant
newPolicy: NewPackagePolicy; | ||
input: Extract<NewPackagePolicyPostureInput, { type: 'cloudbeat/cis_aws' }>; | ||
packageInfo: PackageInfo; | ||
onChange: (opts: any) => void; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…stic#159994) (cherry picked from commit 5728bfa)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
#159994) (#160559) # Backport This will backport the following commits from `main` to `8.9`: - [[CloudSecurity][Fleet] Add CloudFormation install method to CSPM (#159994)](#159994) <!--- Backport version: 8.9.7 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Paulo Henrique","email":"paulo.henrique@elastic.co"},"sourceCommit":{"committedDate":"2023-06-26T18:19:05Z","message":"[CloudSecurity][Fleet] Add CloudFormation install method to CSPM (#159994)","sha":"5728bfa1a7514eeb425e0894dab85ba63f48e8fa","branchLabelMapping":{"^v8.10.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Fleet","release_note:feature","Team:Cloud Security","backport:prev-minor","ci:cloud-deploy","v8.9.0","v8.10.0"],"number":159994,"url":"https://github.com/elastic/kibana/pull/159994","mergeCommit":{"message":"[CloudSecurity][Fleet] Add CloudFormation install method to CSPM (#159994)","sha":"5728bfa1a7514eeb425e0894dab85ba63f48e8fa"}},"sourceBranch":"main","suggestedTargetBranches":["8.9"],"targetPullRequestStates":[{"branch":"8.9","label":"v8.9.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.10.0","labelRegex":"^v8.10.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/159994","number":159994,"mergeCommit":{"message":"[CloudSecurity][Fleet] Add CloudFormation install method to CSPM (#159994)","sha":"5728bfa1a7514eeb425e0894dab85ba63f48e8fa"}}]}] BACKPORT--> Co-authored-by: Paulo Henrique <paulo.henrique@elastic.co>
Summary
it closes #157761
This PR adds the CloudFormation install method to the CSPM Cloud Security integration. The following changes were made:
Cloud Security changes
CloudFormation
orManual
setup format.Updates
aws.credentials.type : cloud_formation
for better backwards compatibilityFleet changes
SUBMITTED_CLOUD_FORMATION
to the integration install form to show the CloudFormation post-install modal for CloudFormation integrations.useCreateCloudFormationUrl
hook.Screenshots