Skip to content

fix to pooler TLS support #2219

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Mar 7, 2023
Merged

fix to pooler TLS support #2219

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f97c665
fix to pooler TLS support, security context fsGroup added (#2216)
2tvenom Feb 13, 2023
468d9c2
add environment variable of CA cert path in pooler pod template
2tvenom Feb 16, 2023
675a474
additional logic for custom CA secrets and mount path
2tvenom Feb 16, 2023
1c418a1
fix ca file name
2tvenom Feb 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 21 additions & 2 deletions pkg/cluster/connection_pooler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,20 +348,33 @@ func (c *Cluster) generateConnectionPoolerPodTemplate(role PostgresRole) (
// Env vars
crtFile := spec.TLS.CertificateFile
keyFile := spec.TLS.PrivateKeyFile
caFile := spec.TLS.CAFile
mountPath := "/tls"
mountPathCA := mountPath

if crtFile == "" {
crtFile = "tls.crt"
}
if keyFile == "" {
keyFile = "tls.key"
}
if caFile == "" {
caFile = "ca.crt"
}
if spec.TLS.CASecretName != "" {
mountPathCA = mountPath + "ca"
}
Copy link
Member

Choose a reason for hiding this comment

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

I believe, we need a second volume mount when introducing CA file support. Hm ... can you remove everything about CA file support from this PR as it looks pretty half baked at the moment and not tested.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i will mount CA


envVars = append(
envVars,
v1.EnvVar{
Name: "CONNECTION_POOLER_CLIENT_TLS_CRT", Value: filepath.Join("/tls", crtFile),
Name: "CONNECTION_POOLER_CLIENT_TLS_CRT", Value: filepath.Join(mountPath, crtFile),
},
v1.EnvVar{
Name: "CONNECTION_POOLER_CLIENT_TLS_KEY", Value: filepath.Join(mountPath, keyFile),
},
v1.EnvVar{
Name: "CONNECTION_POOLER_CLIENT_TLS_KEY", Value: filepath.Join("/tls", keyFile),
Name: "CONNECTION_POOLER_CLIENT_CA_FILE", Value: filepath.Join(mountPathCA, caFile),
},
)

Expand Down Expand Up @@ -402,6 +415,12 @@ func (c *Cluster) generateConnectionPoolerPodTemplate(role PostgresRole) (
},
}

if spec.TLS != nil && spec.TLS.SecretName != "" && spec.SpiloFSGroup != nil {
podTemplate.Spec.SecurityContext = &v1.PodSecurityContext{
FSGroup: spec.SpiloFSGroup,
Copy link
Member

Choose a reason for hiding this comment

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

Why should it get the spilo FS group btw? Can it also get it's own? Would an extra config option make sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you mean additional fsGroup in CRD pooler block?

Copy link
Member

@FxKu FxKu Feb 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

There could be extra settings in the pooler struct, yes. But, on the other hand we're already re-using TLS settings from postgresql.spec - so I'm fine by copying SpiloFSGroup, too, as it would be convenient to users of this feature. No extra config necessary in the manifest.

Was just wondering if somebody might want this FSGroup setting to be different from spilo...

}
}

nodeAffinity := c.nodeAffinity(c.OpConfig.NodeReadinessLabel, spec.NodeAffinity)
if c.OpConfig.EnablePodAntiAffinity {
labelsSet := labels.Set(c.connectionPoolerLabels(role, false).MatchLabels)
Expand Down