fix: Implement custom schema for config.affinity.nodeSelector

[sbernauer]

Description

Fixes stackabletech/issues#554.

IMHO it's not worth an immediate platform bugfix release, as there is a workaround available.

The effect can be seen e.g. in Trino:

diff --git a/deploy/helm/trino-operator/crds/crds.yaml b/deploy/helm/trino-operator/crds/crds.yaml
index e41fdfb..5de8694 100644
--- a/deploy/helm/trino-operator/crds/crds.yaml
+++ b/deploy/helm/trino-operator/crds/crds.yaml
@@ -305,6 +305,8 @@ spec:
                                   type: object
                               type: object
                             nodeSelector:
+                              additionalProperties:
+                                type: string
                               nullable: true
                               type: object
                             podAffinity:
@@ -647,6 +649,8 @@ spec:
                                     type: object
                                   type: array
                               type: object
+                          required:
+                            - nodeSelector
                           type: object

Definition of Done Checklist

• Not all of these items are applicable to all PRs, the author should update this template to only leave the boxes in that are relevant
• Please make sure all these things are done and tick the boxes

Author

Preview Give feedback

1. Changes are OpenShift compatible
2. CRD changes approved
3. Integration tests passed (for non trivial changes)

Reviewer

Preview Give feedback

1. Code contains useful comments
2. (Integration-)Test cases added
3. Documentation added or updated
4. Changelog updated
5. Cargo.toml only contains references to git tags (not specific commits or branches)

Acceptance

Preview Give feedback

1. Feature Tracker has been updated
2. Proper release label has been added

[Techassi]

IMHO it's not worth an immediate bugfix release, as there is a workaround available.

I would be happy to release a new version, because there are now multiple things which are unreleased.

[Techassi]

I would however do one additional PR regarding the changelog before bumping the version.

[sbernauer]

I already changed the wording the "bugfix platform version", sorry for the confusion. No objections against op-rs release whatsoever

[nightkr]

I tried diffing the output before/after this patch: https://gist.github.com/nightkr/9446c9e3747aa6f58ce616cd86c1f13e

As far as I can tell, the actual issue here is that schemars doesn't propagate additionalProperties when used with #[serde(flatten)]? We should probably file an upstream ticket for that and link to it in the code.

[nightkr]

This looks like GREsau/schemars#259.

[nightkr]

GREsau/schemars#275 generates the schema https://gist.github.com/nightkr/f7f06df7a14286a9b682ad6be7e3da14, which should also contain the correct additionalProperties.

[nightkr]

Another workaround is to add #[schemars(deny_unknown_fields)] to StackableNodeSelector: https://gist.github.com/nightkr/af2236b2d3cf77eb71242124d588a638

[sbernauer]

Many thanks for tracking this down @nightkr! What would you suggest as the next step?

1. Merge as is
2. Use #[schemars(deny_unknown_fields)]
3. Wait for a few days to see if Propagate additional_properties when using #[serde(flatten)] GREsau/schemars#275 get's merged
4. Something else

[nightkr]

@sbernauer I'd say 2 and add a tracking issue for 3. schemars doesn't look too lively these days so we shouldn't block on it.

[sbernauer]

Makes sense to me! Adopted Option 2 and created #756.
A quick review would be great, so that @Techassi can release :)

[NickLarsenNZ]

I trust that this works, and as I understand it we simple remove the [schemars(deny_unknown_fields)] as soon as that schemars issue is resolved.