8000 Do not validate CIS on cluster from template by superseb · Pull Request #26887 · rancher/rancher · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

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

Merged
merged 1 commit into from
May 2, 2020

Conversation

superseb
Copy link
Contributor
@superseb superseb commented May 1, 2020

#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 in mgmtclient.Cluster which is also used for the function validateEnforcement. 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.

@superseb superseb marked this pull request as ready for review May 1, 2020 20:45
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")
Copy link
Member

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

Copy link
Contributor

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.

@kinarashah kinarashah merged commit 9b15c4b into rancher:master May 2, 2020
@maggieliu maggieliu requested a review from dramich May 2, 2020 00:41
return httperror.WrapAPIError(err, httperror.InvalidBodyContent, "Cluster spec conversion error")
}

logrus.Tracef("Validator: clusterSpec: %+v\n", &clusterSpec)
Copy link
Contributor

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)
Copy link
Contributor

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.

uhhc pushed a commit to uhhc/rancher that referenced this pull request Sep 29, 2020
…ncheck

Do not validate CIS on cluster from template
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0