Add SubPathExpr option for additionalVolumes

[smutel]

Fix #2464

[smutel]

SubPathExpr and SubPath are mutually exclusive => I don't know how to implement this. Tell me if you have an idea.

[micabe]

Hey @hughcapet,
Can we merge this PR ? It's a useful option to have.
Thanx

[rds13]

👍
Nice addition.

[FxKu]

If SubPath and SubPathExpr are mutually exclusive they cannot coexist here, right?

[FxKu]

This PR is still lacking a few things. The CDR needs to be extended here and here. If the SubPath and SubPathExpr are mutually exclusive you can use OneOf syntax, I think. So far, we are not using it anywhere so you have to search for examples online.

When adding fields to the Postgres CRD, please also run code generation (./hack/update-codegen.sh). And mind that we also have the helm chart, where this new feature has to be addressed, too.

[smutel]

Hello @FxKu,

Thank you for you review.
I did the changes you ask me (I hope).

[smutel]

I am facing this kubernetes/apimachinery#18.

[smutel]

Sounds good now.

[smutel]

Any news on this ?

[FxKu]

@smutel e2e tests are failing. There's an issue with the CRD schema:

spec.validation.openAPIV3Schema.properties[spec].properties[additionalVolumes].items.oneOf[0].properties[subPath].type: Forbidden: must be empty to be structural, 
spec.validation.openAPIV3Schema.properties[spec].properties[additionalVolumes].items.oneOf[1].properties[subPathExpr].type: Forbidden: must be empty to be structural

[smutel]

CRDs updated.

[FxKu]

Btw. I have just noticed that you did not add any logic to the Go code. It's not enough to just add a new field to the CRD. We use a lot of custom logic and not so much K8s specs. In this case the new SubPathExpr muss be set in generateVolumeMounts function. I would like to see a unit test then to check if the new expression is set when specified.

Sorry for suggesting oneOf here as both fields are optional. It's more typical when you have a required type field which specifies different options. You could even put the SubPathExpr in the SubPath field and use an extra bool flag isSubPathExpr. That makes implementation more straight forward. Hope you understand what I mean here.

[smutel]

I changed my PR regarding your suggestion with boolean.
Perhaps the default value for this boolean is missing but I don't know where to specify this.

[smutel]

Looks good to you ?

[smutel]

Any update on this ?

[FxKu]

LGTM now. Sorry for not getting back to this sooner.

[FxKu]

👍

[idanovinda]

👍