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

Cis k8s name migration #3113

Merged
merged 4 commits into from
Apr 20, 2022
Merged

Conversation

kfirpeled
Copy link
Contributor

What does this PR do?

This pr is part of a larger change to rename the current integration of cis_kubernetes_benchmark to cloud_security_posture as it will contain more benchmarks in the future and should have a proper naming.
A followup PR will be to remove the cis_kubernetes_benchmark package once all code base in the different repos will be aligned to the new naming

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.

Author's Checklist

  • [ ]

How to test this PR locally

Related issues

Screenshots

@elasticmachine
Copy link

elasticmachine commented Apr 17, 2022

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2022-04-17T15:09:42.715+0000

  • Duration: 13 min 36 sec

Test stats 🧪

Test Results
Failed 0
Passed 2
Skipped 0
Total 2

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

@kfirpeled kfirpeled added the Team:Cloud Security Label for the Cloud Security team label Apr 17, 2022
@kfirpeled kfirpeled marked this pull request as ready for review April 17, 2022 10:09
@kfirpeled kfirpeled requested review from a team as code owners April 17, 2022 10:09
@eyalkraft
Copy link
Contributor

Maybe a rename is possible instead of this duplication - the package only exists in snapshot environment.
Maybe @mtojek can tell us more.

Additionally I think it's a wrong name for this integration package that we will have to refactor again.
cloud_security_posture is too general - we only do KSPM currently, and expecting that all our future products will be packed in a single integration package is wrong IMO and could create future complications.

@amitkanfer
Copy link
Contributor

and expecting that all our future products will be packed in a single integration package is wrong IMO and could create future complications.

@eyalkraft why do you think it's wrong?
What do you think is the right way to go?

@kfirpeled
Copy link
Contributor Author

kfirpeled commented Apr 17, 2022

Maybe a rename is possible instead of this duplication - the package only exists in snapshot environment.

I don't want to rename before we update kibana, cloudbeat and the automation (not 100% sure about that automation needs to be updated) to use the new name. As it can cause breaking changes. therefore I duplicate it as a migration phase.

@eyalkraft
Copy link
Contributor

I don't think it's inherently wrong but it has some implications.
For example every user gets all of our rule templates so we need some logic in Kibana to only create the relevant concrete rules for the specific benchmark the user wants.
We just need to understand the full implications and open the relevant issues.

packages/cloud_security_posture/changelog.yml Outdated Show resolved Hide resolved
Co-authored-by: eyalkraft <63912106+eyalkraft@users.noreply.github.com>
@kfirpeled
Copy link
Contributor Author

kfirpeled commented Apr 17, 2022

I don't think it's inherently wrong but it has some implications. For example every user gets all of our rule templates so we need some logic in Kibana to only create the relevant concrete rules for the specific benchmark the user wants. We just need to understand the full implications and open the relevant issues.

this is on point, the rule templates will all be saved (which i think is ok), but we will need to distinguish between policy_templates. which I named as kspm. so this should be added on our hook.
I'll go and investigate it rn

@kfirpeled
Copy link
Contributor Author

@eyalkraft, @amitkanfer I've tested it, and we can distinguish between different tiles being installed.

This is the test I've taken:

format_version: 1.0.0
name: cloud_security_posture
-title: "CIS Kubernetes Benchmark"
+title: "CIS Security Posture"
policy_templates:
  - name: kspm
    title: CIS Kubernetes Benchmark
    description: Check Kubernetes cluster compliance with the Kubernetes CIS benchmark.
    multiple: false
    inputs:
      - type: cloudbeat
        title: Enable CIS Kubernetes Benchmark
        description: Collecting findings
+  - name: cspm
+    title: CIS Cloud Benchmark
+    description: testing third tile
+    multiple: false
+    inputs:
+      - type: cloudbeat
+        title: Enable CIS Cloud Benchmark
+        description: Collecting findings

And I examined it during debug and you can see different inputs and their enabled status:
Screen Shot 2022-04-17 at 18 37 33

@eyalkraft
Copy link
Contributor

Nice, so I think in this case in order to keep our kibana plugin free from strings representing the names of our benchmarks we would want to add a field for our rule templates to indicate which policy template(s?) they belong to.

@kfirpeled kfirpeled merged commit d2b6e24 into elastic:main Apr 20, 2022
@kfirpeled kfirpeled deleted the cis_k8s_name_migration branch April 20, 2022 14:07
v1v added a commit to v1v/integrations that referenced this pull request Apr 25, 2022
…ations into feature/labels-cloud-ephemeral-components

* 'feature/labels-cloud-ephemeral-components' of github.com:v1v/integrations: (23 commits)
  use branch
  standardise labels/tags
  Update .ci/Jenkinsfile
  [ci][terraform][aws] tags with metadata
  Bump github.com/elastic/elastic-package from 0.46.0 to 0.47.0 (elastic#3182)
  Spring boot package [Memory - data stream] (elastic#2979)
  [cisco_ise] Add Cisco ISE package (elastic#2855)
  [apache_spark][executor] Add Apache Spark package with Executor data stream (elastic#2943)
  [apache_spark][driver] Add Apache Spark package with Driver data stream (elastic#2945)
  cisco_duo: simplify grok expression for handling ports (elastic#3170)
  cisco_duo: fix handling of IP addresses with port numbers (elastic#3117)
  [sophos] Various improvements and log samples from 18.5 (elastic#3127)
  [cisco_asa] fix visualizations (elastic#3146)
  [apache_spark][application] Add Apache Spark package with Application data stream (elastic#2941)
  Fix pagination bug that skipped events when more than one page is present. (elastic#3140)
  Cis k8s name migration (elastic#3113)
  remove exported fields; these will be added to Kibana docs instead (elastic#3093)
  Exclude ecosystem as owner of subdirectories of packages (elastic#3132)
  [awsfargate] bump package version (elastic#3130)
  add first csp rule template (elastic#3081)
  ...
{
"attributes": {
"description": "",
"title": "logs-cloud_security_posture.findings-*"
Copy link
Contributor

@oren-zohar oren-zohar Apr 25, 2022

Choose a reason for hiding this comment

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

Suggested change
"title": "logs-cloud_security_posture.findings-*"
"title": "logs-cloud_security_posture.findings*"

@kfirpeled I know it's already merged, but since the current findings index is logs-cloud_security_posture.findings_latest-default this data view didn't catch it 😢

Copy link
Contributor

Choose a reason for hiding this comment

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

@oren-zohar This is on purpose, we'll move the findings_latest index in a different PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Cloud Security Label for the Cloud Security team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants