Fields immutable #2070
Fields immutable #2070
Conversation
Signed-off-by: pjuarezd <pjuarezd@users.noreply.github.com>
Signed-off-by: pjuarezd <pjuarezd@users.noreply.github.com>
…add validations Signed-off-by: pjuarezd <pjuarezd@users.noreply.github.com>
|
Looks good to me |
Signed-off-by: pjuarezd <pjuarezd@users.noreply.github.com>
| @@ -609,18 +611,19 @@ type CustomCertificateConfig struct { | |||
| // | |||
| // See the https://min.io/docs/minio/kubernetes/upstream/operations/install-deploy-manage/deploy-minio-tenant.html#procedure-command-line[MinIO Operator CRD] reference for the `pools` object for examples and more complete documentation. + | |||
| type Pool struct { | |||
| // *Optional* + | |||
| // *Required* | |||
There was a problem hiding this comment.
This may break legacy pools without a name, we should document this change
There was a problem hiding this comment.
Not may. It will break existing pools:
failed to create manager for existing fields: failed to convert new object (minio-private/minio; minio.min.io/v2, Kind=Tenant) to smd typed: .spec.pools: element 0: associative list with keys has an element that omits key field "name" (and doesn't have default value)
I don't get why this change was approved and even released already (btw. as a bug fix release - do you follow SemVer?) without any existing documentation. Sorry to say, but the entire release process for the operator is somehow broken.
There was a problem hiding this comment.
This PR contains a lot of formatting changes. There are also some changes that don't seem to be mentioned in the PRs description.. It may be a good idea to do do styling changes in a separate PR to make reviewing easier. I think we should also be aware that we don't push auto-formatted files, because of different editor settings. Either we should synchronize them or ignore it when checking in.
| |*Required* + | ||
|
|
||
|
|
||
| TenantRef Reference for minio Tenant to eun the jobs against |
There was a problem hiding this comment.
| TenantRef Reference for minio Tenant to eun the jobs against | |
| TenantRef Reference for minio Tenant to run the jobs against |
| |*`mcImage`* __string__ | ||
| |mc job image |
There was a problem hiding this comment.
Not sure why this is in this PR.
| @@ -101,6 +107,51 @@ CertificateStatus keeps track of all the certificates managed by the operator | |||
| |=== | |||
|
|
|||
|
|
|||
| [id="{anchor_prefix}-github-com-minio-operator-pkg-apis-minio-min-io-v2-customcertificateconfig"] | |||
There was a problem hiding this comment.
I guess this is not part of this PR.
What does it do?
Adds validations in the fields
spec.pools.*.volumesPerServerandspec.pools.*.serversto make the fields read-only after being created.This could be helpful to prevent undesired changes on
volumesPerServerandserversthat could cause a Pool to not start.As a side-effect the field
spec.pools.*.nameneeded to be changed fromOptionaltoRequired, to overcome a limitation on OpenAPI.How does it look?
On creation the fields are writtable
On edition are not editable anymore, a validation error as following will be shown: