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

get hardening policies for replicasets, statefulsets & daemonsets #681

Merged
merged 4 commits into from Apr 25, 2023

Conversation

Prateeknandle
Copy link
Contributor

@Prateeknandle Prateeknandle commented Mar 9, 2023

fixes #670

changes:

  • getting replicasets, daemonsets and statefulsets from k8s client and generating hardening policies respectively
  • updating clusterRole for the same
  • added test cases to test whether we get the hardening policy or not

@Prateeknandle Prateeknandle changed the title Get hardening pol [Bug] get hardening policies for replicasets & statefulsets Mar 9, 2023
@vishnusomank
Copy link
Contributor

LGTM!

@Prateeknandle Can we update the helm chart as well to match the ClusterRole ?

@Prateeknandle
Copy link
Contributor Author

@Prateeknandle Can we update the helm chart as well to match the ClusterRole ?

updated

Copy link
Contributor

@seswarrajan seswarrajan left a comment

Choose a reason for hiding this comment

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

LGTM

@Prateeknandle
Copy link
Contributor Author

can we merge the pr?

@nyrahul
Copy link
Contributor

nyrahul commented Mar 10, 2023

can we merge the pr?

The test is failing!

@nyrahul
Copy link
Contributor

nyrahul commented Mar 10, 2023

Can we add a testcase to test this?

@Prateeknandle
Copy link
Contributor Author

Prateeknandle commented Mar 10, 2023

Can we add a testcase to test this?

tests for recommend are defined in kubearmor-client

The test is failing!

it's because the tests are flaky

For testing whether changes are working or not we can see logs

Copy link
Contributor

@vishnusomank vishnusomank left a comment

Choose a reason for hiding this comment

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

LGTM!

Tested by deploying via deployment.yaml and helm. The code changes are able to detect and generate policies for Statefulset and Replicaset.
Only issue is if a new replicaset or statefulset is deployed we'll have to wait for the cronjob to trigger (every 1 hour) to get hardening policies for those.

@Prateeknandle
Copy link
Contributor Author

Prateeknandle commented Mar 22, 2023

With the new changes I've checked locally, tests are passing and discovery-engine is able to get the hardening policies.

the ginkgo test which are failing are the flaky ones and not related to this pr.

@Prateeknandle Prateeknandle changed the title [Bug] get hardening policies for replicasets & statefulsets get hardening policies for replicasets & statefulsets Mar 23, 2023
src/recommendpolicy/recommendPolicy.go Outdated Show resolved Hide resolved
src/recommendpolicy/recommendPolicy.go Outdated Show resolved Hide resolved
src/recommendpolicy/recommendPolicy.go Show resolved Hide resolved
src/recommendpolicy/recommendPolicy.go Show resolved Hide resolved
@Ankurk99
Copy link
Contributor

Hey @Prateeknandle, Can you please resolve the conflicts in the PR?

Signed-off-by: Prateeknandle <prateeknandle@gmail.com>
Prateeknandle and others added 2 commits April 18, 2023 18:58
Signed-off-by: Prateeknandle <prateeknandle@gmail.com>
- add check for owner reference in case of deployment and statefulsets
- show downloaded policy-template version

Signed-off-by: Ankur Kothiwal <ankur.kothiwal99@gmail.com>
Copy link
Contributor

@vishnusomank vishnusomank left a comment

Choose a reason for hiding this comment

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

I think we should handle daemonsets as well in this. @Ankurk99 what do you think?

@Ankurk99 Ankurk99 changed the title get hardening policies for replicasets & statefulsets get hardening policies for replicasets, statefulsets & daemonsets Apr 20, 2023
Copy link
Contributor

@seswarrajan seswarrajan left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@nyrahul nyrahul left a comment

Choose a reason for hiding this comment

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

Mostly good. Just one small fix.

src/recommendpolicy/recommendPolicy.go Outdated Show resolved Hide resolved
Signed-off-by: Ankur Kothiwal <ankur.kothiwal99@gmail.com>
@nyrahul
Copy link
Contributor

nyrahul commented Apr 25, 2023

@vishnusomank , can you provide your approval?

@nyrahul nyrahul merged commit 4dfdcd1 into accuknox:dev Apr 25, 2023
5 of 6 checks passed
Comment on lines +165 to 167
for _, ns := range nsNotFilter {
if d.Namespace != ns {
generateHardenPolicy(d.Name, d.Namespace, d.Spec.Template.Labels)
Copy link
Contributor

Choose a reason for hiding this comment

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

We are already generating policies for Deployments here. Why are we generating them again at line 150? It should be in a single place only.

Comment on lines +226 to +233
if !isLatest() {
version, err := DownloadAndUnzipRelease()
if err != nil {
log.Error().Msgf("Unable to download %v", err.Error())
return nil
}
log.Info().Msgf("Downloaded version: %v", version)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This should not be clubbed with GetHardenPolicy(...). Downloading and unzipping release is a separate process. Policy templates are required for both KubeArmor and Kyverno policies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hardening policies only getting generated for deployments
6 participants