SAP-archive / karydia

Kubernetes Security Walnut
Other
77 stars 10 forks source link

Use the "--namespace" CLI flag #227

Closed Neumann-Nils closed 5 years ago

Neumann-Nils commented 5 years ago

Description

This PR removes the YAML file that creates the "karydia" namespace during the installation process and let Helm handle the namespace via the --namespace CLI flag.

Resolves #224.

Checklist

Before submitting this PR, please make sure:

Neumann-Nils commented 5 years ago

Wanted some feedback on the discussion in #224.

ionysos commented 5 years ago

Is it possible to just use {{ .Release.Namespace }} as "placeholder" for the namespace at the helm template YAMLs? If I understood it correctly, this represents the current helm setting which can also be modified via the --namespace CLI flag. With that we don't need a hardcoded namespace name or a specification in the values.yaml and I think it's not that confusing anymore. Moreover, we could keep the karydia-namespace.yaml file.

Neumann-Nils commented 5 years ago

Did not know that the CLI flag is accessible via {{ .Release.Namespace }}. But it works great! This is definitely the better solution, as it keeps the value consistent.

I don't think we can keep the karydia-namespace.yaml. To my understanding, Helm will always try to install the namespace set in the CLI flag if it does not exist. If the installation starts, Helm will not find the namespace karydia and creates it (see https://github.com/helm/helm/issues/4456#issuecomment-412134651). Afterwards, it will apply the YAML files. The karydia-namespace.yaml will try to create the namespace karydia and will fail (as the namespace karydia was already created by Helm in the step before). @ionysos do you have the same understanding of this process?

ionysos commented 5 years ago

Did not know that the CLI flag is accessible via {{ .Release.Namespace }}. But it works great! This is definitely the better solution, as it keeps the value consistent.

I don't think we can keep the karydia-namespace.yaml. To my understanding, Helm will always try to install the namespace set in the CLI flag if it does not exist. If the installation starts, Helm will not find the namespace karydia and creates it (see helm/helm#4456 (comment)). Afterwards, it will apply the YAML files. The karydia-namespace.yaml will try to create the namespace karydia and will fail (as the namespace karydia was already created by Helm in the step before). @ionysos do you have the same understanding of this process?

Yes. Looks like we don't need the karydia-namespace.yaml anymore. Thanks for double-checking that. 👍

Neumann-Nils commented 5 years ago

Checked the files you mentioned:

docs/demos/custom_seccomp.md only describes how to manual create and distribute custom seccomp profiles. Thus, there is no need to change the used namespace (though one could change it to default or similar).

The scripts in the /scripts folder are not used by the installation (this is true for the Helm and the manual installation). I am not sure if there is still a use-case that use them. I believe in the beginning we had different installation processes for production, development and testing. Maybe these are artefacts from this migration process. Or are they still used? Otherwise we could delete them.

ionysos commented 5 years ago

Or are they still used?

I'm not sure about that but my feeling is that we don't need them anymore if we don't need them for our (new &) current installation process.