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

chore: support storageClassName for trivy-server statefulset #977

Merged

Conversation

MaxFedotov
Copy link
Contributor

Description

Add support for storageClassName param for trivy-server statefulset

Related issues

Remove this section if you don't have related PRs.

Checklist

  • I've read the guidelines for contributing to this repository.
  • I've added tests that prove my fix is effective or that my feature works.
  • I've updated the documentation with the relevant information (if needed).
  • I've added usage information (if the PR introduces new options)
  • I've included a "before" and "after" example to the description (if the PR is a user interface change).

@CLAassistant
Copy link

CLAassistant commented Feb 15, 2023

CLA assistant check
All committers have signed the CLA.

@MaxFedotov MaxFedotov force-pushed the trivy_server_storageClassName branch from ed2e1d7 to df5b663 Compare February 15, 2023 15:15
Copy link
Contributor

@fengshunli fengshunli left a comment

Choose a reason for hiding this comment

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

LGTM

@chen-keinan
Copy link
Contributor

@MaxFedotov thank you for the contribution. could you please update docs ( trivy and helm )

@fengshunli
Copy link
Contributor

fengshunli commented Feb 15, 2023

@MaxFedotov thank you for the contribution. could you please update docs ( trivy and helm )

I'll deal with it tomorrow @chen-keinan

@chen-keinan
Copy link
Contributor

@MaxFedotov thank you for the contribution. could you please update docs ( trivy and helm )

I'll deal with it tomorrow @chen-keinan

It should be included as part of this PR @MaxFedotov

@MaxFedotov
Copy link
Contributor Author

@chen-keinan ok, will add it.

@fengshunli i've also found out that trivy-server uses serviceAccountName: trivy-operator. But this service account is not created by helm. Is it ok if I will add it in the same PR?

@chen-keinan
Copy link
Contributor

chen-keinan commented Feb 15, 2023

@chen-keinan ok, will add it.

@fengshunli i've also found out that trivy-server uses serviceAccountName: trivy-operator. But this service account is not created by helm. Is it ok if I will add it in the same PR?

@MaxFedotov the service accout is created in helm

@fengshunli
Copy link
Contributor

Update the md, add a set parameter, and check it @MaxFedotov

@MaxFedotov
Copy link
Contributor Author

@chen-keinan

but the thing is that its name is

name: {{ include "trivy-operator.serviceAccountName" . }}

which is generated as

{{/*
Create the name of the service account to use.
*/}}
{{- define "trivy-operator.serviceAccountName" -}}
{{- if .Values.serviceAccount.create }}
{{- default (include "trivy-operator.fullname" .) .Values.serviceAccount.name }}
{{- else }}
{{- default "default" .Values.serviceAccount.name }}
{{- end }}
{{- end }}

but in statefulset it is hardcoded as

serviceAccountName: trivy-operator

what I wanted to do is to set it to

serviceAccountName: {{ include "trivy-operator.serviceAccountName" . }}

the same way, as for operator deployment

@chen-keinan
Copy link
Contributor

@MaxFedotov sound good, you can make it in same PR if you wish

@MaxFedotov
Copy link
Contributor Author

@chen-keinan done

@MaxFedotov
Copy link
Contributor Author

and one more thing I would like to add - support for nodeSelector for trivy-server

Copy link
Contributor

@fengshunli fengshunli left a comment

Choose a reason for hiding this comment

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

test

@fengshunli
Copy link
Contributor

and one more thing I would like to add - support for nodeSelector for trivy-server

Will the affinity be better

@MaxFedotov
Copy link
Contributor Author

That depends on the use-case :) we use nodeSelector to schedule pod on correct tenant nodes

@chen-keinan
Copy link
Contributor

lgtm 🚀

@chen-keinan chen-keinan merged commit 8be21d3 into aquasecurity:main Feb 15, 2023
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.

Helm Chart should allow specification of trivy server storage class
4 participants