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: reformat chart templates #1439

Merged
merged 1 commit into from
Aug 19, 2023

Conversation

dnskr
Copy link
Contributor

@dnskr dnskr commented Aug 15, 2023

Description

The PR reformats chart templates to common style with more readable blocks and indents.
ConfigMap trivy-operator-policies-config has been deleted, because it does not have any data.

The PR does not change any functionality of the chart.
Similar PRs: apache/airflow#29917 apache/airflow#29941 apache/airflow#30312 minio/minio#16947 apache/superset#23681

Test

The following command prints same (minor differences) output before and after changes:

helm template test deploy/helm

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).

@dnskr dnskr force-pushed the reformat_chart_templates branch 2 times, most recently from 293875d to 146b671 Compare August 15, 2023 22:14
@chen-keinan
Copy link
Collaborator

@dnskr lgtm , just once comment , why you decided to delete the policies.yaml ?

@dnskr
Copy link
Contributor Author

dnskr commented Aug 16, 2023

I deleted policies.yaml file, because it contains ConfigMap trivy-operator-policies-config only which is constantly empty.
I mean there are no keys in it and the keys cannot be added through values.

@chen-keinan
Copy link
Collaborator

I deleted policies.yaml file, because it contains ConfigMap trivy-operator-policies-config only which is constantly empty. I mean there are no keys in it and the keys cannot be added through values.

this configmap is a place holder for for external rego policies for configAudit reports (empty by default), as built-in policies are used by default.

it is used by our customers and users, Lets keep it for now.

@dnskr
Copy link
Contributor Author

dnskr commented Aug 17, 2023

I'm ok to keep it for now. However, could you please explain how the ConfigMap trivy-operator-policies-config is used by users? I'm asking, because of the following:

  • The configmap is empty and it's impossible to populate it using Helm and command like helm install ... --set secretKey=secretValue.
  • There are no resources in the chart which refer to trivy-operator-policies-config, i.e. the configmap is not mounted to any container filesystem and it's not used as the source for environment variable. So, I don't understand how it can be used then.
  • If the users populate it manually with kubectl edit ..., then the content of the configmap will be overwritten/wiped during next chart release upgrade helm upgrade ....

I found one more empty resource - Secret trivy-operator in config.yaml file.

@chen-keinan
Copy link
Collaborator

I'm ok to keep it for now. However, could you please explain how the ConfigMap trivy-operator-policies-config is used by users? I'm asking, because of the following:

  • The configmap is empty and it's impossible to populate it using Helm and command like helm install ... --set secretKey=secretValue.
  • There are no resources in the chart which refer to trivy-operator-policies-config, i.e. the configmap is not mounted to any container filesystem and it's not used as the source for environment variable. So, I don't understand how it can be used then.
  • If the users populate it manually with kubectl edit ..., then the content of the configmap will be overwritten/wiped during next chart release upgrade helm upgrade ....

I found one more empty resource - Secret trivy-operator in config.yaml file.

I agree that its can't be update via helm, however it is used by users who update it with kubectl and hooks on every upgrade.
users also use it via trivy-operator static yaml.

in addition we have plan for it in the future therefore I prefer to keep it

@dnskr
Copy link
Contributor Author

dnskr commented Aug 18, 2023

Got it! I returned it back.

@chen-keinan
Copy link
Collaborator

@dnskr thank you for the contribution lgtm 🚀

@chen-keinan chen-keinan merged commit 40977b1 into aquasecurity:main Aug 19, 2023
7 checks passed
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.

None yet

2 participants