WIP: Refactoring certs and volumes for sts

[sergeimonakhov]

This pull request introduces code changes related to managing volumes for the STS object. Key modifications include the addition of structures to store certificate information and the definition of paths to certificate files. Additionally, changes have been made to the creation and mounting of the data volume for the ETCD container. These alterations aim to enhance certificate handling and data management within the controller's context.

[sircthulhu]

@sergeimonakhov
Please run make lint-fix to resolve pre-commit errors and fix tests

[sergeimonakhov]

@sircthulhu I added even more refactoring and fixed the code to make the tests run. Overall, the base code was very poor; we need to be more stringent in our code reviews because it will become impossible to maintain if we keep accepting sloppy work.

[aobort]

I'd suggest to use copy:

volumeMounts := make([]corev1.VolumeMount, len(list))
copy(volumeMounts, list)

[sergeimonakhov]

I'd suggest to use copy:

volumeMounts := make([]corev1.VolumeMount, len(list))
copy(volumeMounts, list)

It won't work that way because the function mergeVolumeMounts accepts ...[]corev1.VolumeMount as an argument, which allows passing N slices. While it's possible to calculate the total length of the slices, that would require a loop. In this case, using copy versus append would make no difference.

[lllamnyp]

Or simply back on line 196

volumeMounts := append(tlsVolumeMounts, dataVolumeMount...)

[sergeimonakhov]

Or simply back on line 196

volumeMounts := append(tlsVolumeMounts, dataVolumeMount...)

Ok, How would it look if you need to add a third list for merging?
The mergeVolumeMounts function is specifically designed to be universal, allowing the merging of N lists.

[aobort]

This function is used in the only place, function which accepts slice as an argument and merges this slice with computed slice in its body.
IMO, it makes no sense to write dedicated function for merging slices of concrete type which accepts variadic number of arguments if it is used in one place where the number of arguments which will be passed to that function is fixed.

[aobort]

It won't work that way because the function mergeVolumeMounts accepts ...[]corev1.VolumeMount as an argument, which allows passing N slices. While it's possible to calculate the total length of the slices, that would require a loop. In this case, using copy versus append would make no difference.

Yes, my bad. Missed that the argument is not a slice, but the slice of slices

[sergeimonakhov]

This function is used in the only place, function which accepts slice as an argument and merges this slice with computed slice in its body. IMO, it makes no sense to write dedicated function for merging slices of concrete type which accepts variadic number of arguments if it is used in one place where the number of arguments which will be passed to that function is fixed.

Initially, there were no certificates, but then they appeared. Now, let's say some config maps and who knows what else will appear later. How would the code look then? Right now, it's very simple: we just add slices to volumeMounts := mergeVolumeMounts(tlsVolumeMounts, dataVolumeMount, configMapVolumeMount, etcVolumeMount).

[aobort]

Initially, there were no certificates, but then they appeared. Now, let's say some config maps and who knows what else will appear later. How would the code look then? Right now, it's very simple: we just add slices to volumeMounts := mergeVolumeMounts(tlsVolumeMounts, dataVolumeMount, configMapVolumeMount, etcVolumeMount).

In case of any volume mounts defined by the user, I would expect is that function generateVolumeMounts will accept as arguments:

1. predefined mounts - data, tls certs, whatnot else
2. container volumeMounts defined in cluster spec by the user

and merge these two lists. Computation of predefined mounts is the separate step where we need to take into account provided cluster configuration, like security config.
Bam, profit.

[kvaps]

closed in favor #224