馃尡 Add Cluster and ClusterClass variable defaulting and validation #5615
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
k8s-ci-robot
added
do-not-merge/work-in-progress
k8s-ci-robot
added
the
size/XL
killianmuldoon
changed the title
k8s-ci-robot
removed
the
do-not-merge/work-in-progress
sbueringer
reviewed
Nov 16, 2021
k8s-ci-robot
added
the
needs-rebase
4 tasks
sbueringer
reviewed
Nov 19, 2021
killianmuldoon
force-pushed
the
pr-clusterclass-variable-validation-defaulting
branch
from
d279ff8
to
c7e1785
Compare
k8s-ci-robot
added
size/XXL
docs/proposals/202105256-cluster-class-and-managed-topologies.md
Outdated
Show resolved
Hide resolved
|
|
||
| // DefaultClusterVariables defaults variables which do not exist in clusterVariable, if they | ||
| // have a default value in the corresponding schema in clusterClassVariable. | ||
| func DefaultClusterVariables(clusterVariables []clusterv1.ClusterVariable, clusterClassVariables []clusterv1.ClusterClassVariable, fldPath *field.Path) ([]clusterv1.ClusterVariable, field.ErrorList) { |
There was a problem hiding this comment.
Suggested change
| func DefaultClusterVariables(clusterVariables []clusterv1.ClusterVariable, clusterClassVariables []clusterv1.ClusterClassVariable, fldPath *field.Path) ([]clusterv1.ClusterVariable, field.ErrorList) { | |
| func DefaultClusterVariables(clusterVariables []clusterv1.ClusterVariable, clusterClassVariables []clusterv1.ClusterClassVariable) ([]clusterv1.ClusterVariable, field.ErrorList) { |
all the checks in this func are referring to to spec.topology.variables.
There was a problem hiding this comment.
Just felt like a best practice to pass in the relative field path which fits the struct we pass in which is then further amended. It makes it a lot easier to track per function if the path is constructed correctly
There was a problem hiding this comment.
internal/topology/variables/clusterclass_variable_validation.go
Outdated
Show resolved
Hide resolved
k8s-ci-robot
added
the
needs-rebase
killianmuldoon
force-pushed
the
pr-clusterclass-variable-validation-defaulting
branch
from
c7e1785
to
2ebd776
Compare
k8s-ci-robot
removed
the
needs-rebase
killianmuldoon
force-pushed
the
pr-clusterclass-variable-validation-defaulting
branch
6 times, most recently
from
ee697dd
to
d8dc4e3
Compare
sbueringer
reviewed
Nov 24, 2021
internal/topology/variables/clusterclass_variable_validation.go
Outdated
Show resolved
Hide resolved
internal/topology/variables/clusterclass_variable_validation.go
Outdated
Show resolved
Hide resolved
killianmuldoon
force-pushed
the
pr-clusterclass-variable-validation-defaulting
branch
from
d8dc4e3
to
cd38690
Compare
killianmuldoon
changed the title
killianmuldoon
force-pushed
the
pr-clusterclass-variable-validation-defaulting
branch
from
bca111b
to
67076b5
Compare
killianmuldoon
force-pushed
the
pr-clusterclass-variable-validation-defaulting
branch
from
067f668
to
7ccd337
Compare
sbueringer
reviewed
Nov 24, 2021
docs/proposals/202105256-cluster-class-and-managed-topologies.md
Outdated
Show resolved
Hide resolved
killianmuldoon
force-pushed
the
pr-clusterclass-variable-validation-defaulting
branch
from
0211793
to
14ab981
Compare
sbueringer
reviewed
Nov 24, 2021
internal/topology/variables/cluster_variable_defaulting_test.go
Outdated
Show resolved
Hide resolved
killianmuldoon
force-pushed
the
pr-clusterclass-variable-validation-defaulting
branch
2 times, most recently
from
141f967
to
fbc1117
Compare
Signed-off-by: Stefan B眉ringer buringerst@vmware.com Signed-off-by: killianmuldoon <kmuldoon@vmware.com> Co-authored-by: killianmuldoon kmuldoon@vmware.com
killianmuldoon
force-pushed
the
pr-clusterclass-variable-validation-defaulting
branch
from
fbc1117
to
9099eb5
Compare
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fabriziopandini The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
sbueringer
deleted the
pr-clusterclass-variable-validation-defaulting
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Signed-off-by: Stefan B眉ringer buringerst@vmware.com
Introduce webhook functions to validate the variables contained in a Cluster against their definitions in a ClusterClass, validate that the definitions of variables in a clusterClass are valid, and default the variables in a Cluster based on the defaulting defined in their schemas.
Fixes #5612