spilo_fsgroup: set to 103 by default

[zimbatm]

Setting the FSGroup to match the postgres GID is a requirement for postgres to load the TLS certificates mounted on a secret volume (which will be handled by #690).

So now the question is, why not always set the FSGroup to match the postgres GID?

[FxKu]

Please, add the default for SpiloFSGroup also in the internal struct.

Not sure if I remember the discussion from #531 correctly: Does this work out-of-the-box or is it required to patch the Spilo image? @erthalion

[zimbatm]

I have a local kind setup to test these changes. I only tested that the master and slave come up properly. Let me know if you want me to do some more testing.

[zimbatm]

@FxKu the PR is ready for another review

[zimbatm]

rebased

[erthalion]

@zimbatm Could there be any other use cases for this except mentioned in #690 ? The change by itself is fine, but can come with compatibility issues if someone would try to use an image different from Spilo (e.g. one of it's modifications) and system gid for postgres could be different from 103.

[zimbatm]

How I see things, the operator is tightly coupled with the spilo image and if the user does something custom they would have to override the default gid as well. Most likely their custom spilo image would be a fork from the official spilo project and inherit the gid.

The motivation of this change is to make the TLS setup a tiny bit easier. I don't have other scenarios in mind that could also benefit from this change. Although I suspect that any other volume that is mounted in the spilo pod would probably want to be readable from the postgres group.

[erthalion]

How I see things, the operator is tightly coupled with the spilo image

Sort of, but tight coupling doesn't mean it's not possible to use some modifications, and how far those modifications are coming is up to an author. I would also mention that this is a pod-level parameter, which means all sidecars will also have it. Overall since all this is only for one feature, changing the default value seems questionable for me.

But probably it makes sense to go other way around. I don't see why we can't say in #690 that if we have custom TLS section defined, then also set spilo_fsgroup to some predefined default value, which for now can be just a constant suitable for the original spilo image. What do you think?

[zimbatm]

Alright. Thanks for looking into this and replying to my questions. I was hoping that this change would simplify the TLS setup and ultimately remove one if-condition branch in the whole system. But like you said, the security context is applied to the whole pod unfortunately and I don't see another route. #690 already checks to make sure that the FSGroup is being set so I will push that to the TLS configuration documentation instead.

[erthalion]

Thank you.

I will push that to the TLS configuration documentation instead.

But actually I've meant that we can set spilo_fsgroup in this case automatically, without requiring to modify operator configuration. At the end of the day operator generates a pod manifest, so why not set securityContext.FSGroup there?

[zimbatm]

Okay