-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Do not validate CIS on cluster from template #26887
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.
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
Do not validate CIS on cluster from template #26887
Conversation
var clientClusterSpec mgmtclient.Cluster | ||
logrus.Tracef("Validator: data: %+v\n", data) | ||
if err := convert.ToObj(data, &clusterSpec); err != nil { | ||
return httperror.WrapAPIError(err, httperror.InvalidBodyContent, "Cluster spec conversion error") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: we'd need different err msg for client and cluster spec
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed here, error needs to be updated.
return httperror.WrapAPIError(err, httperror.InvalidBodyContent, "Cluster spec conversion error") | ||
} | ||
|
||
logrus.Tracef("Validator: clusterSpec: %+v\n", &clusterSpec) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't log this, it can/will have creds in it. Logging entire objects even in trace is not safe.
var spec v3.ClusterSpec | ||
var clusterSpec v3.ClusterSpec | ||
var clientClusterSpec mgmtclient.Cluster | ||
logrus.Tracef("Validator: data: %+v\n", data) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This log can leak as well.
…ncheck Do not validate CIS on cluster from template
#26616
We got a panic when a cluster was validated that was created from a cluster template as it does not contain version information when posted to the API. The initial thought was to check for
spec.RancherKubernetesEngineConfig
being nil but there is a field when a cluster template is used (ClusterTemplateRevisionID
) but this wasn't available in the type that was used (v3.ClusterSpec
) but is available inmgmtclient.Cluster
which is also used for the functionvalidateEnforcement
. So I changed the type and used that, only thing I ran into is that the field for ProfileType are strings in the other type so I have to cast them.