Identify cluster scope by a boolean field rather than a special namespace

This commit is contained in:
Mike Spreitzer
2019-11-13 10:32:50 -05:00
parent 3b77bc8054
commit a912bd8488
4 changed files with 46 additions and 65 deletions

View File

@@ -29,7 +29,6 @@ const (
NameAll = "*"
NamespaceEvery = "*" // matches every particular namespace
NamespaceClusterScope = "Cluster Scope" // matches absence of namespace
)
// System preset priority level names
@@ -240,29 +239,23 @@ type ResourcePolicyRule struct {
// +listType=set
Resources []string
// `clusterScope` indicates whether to match requests that do not
// specify a namespace (which happens either because the resource
// is not namespaced or the request targets all namespaces).
// If this field is omitted or false then the `namespaces` field
// must contain a non-empty list.
// +optional
ClusterScope bool
// `namespaces` is a list of target namespaces that restricts
// matches. A request that does not specify a target namespace
// (which happens both when the resource is not namespaced and
// when the resource is namespaced and the request is for all
// namespaces) matches only if this list includes "Cluster Scope"
// (this string is not a valid namespace and thus can not be
// confused with an actual namespace). A request that specifies a
// target namespace matches only if either (a) this list contains
// that target namespace or (b) this list contains "*".
//
// This list may not be omitted or empty. If the list contains
// "*" then the only other allowed member is "Cluster Scope".
// Without "*", it is allowed to list "Cluster Scope" along with
// particular namespaces.
//
// Requests will match only if the values in this list are
// appropriate for the resource(s) involved. For example: for a
// cluster scoped resource (i.e., one not namespaced) a request
// can match only if this list contains "Cluster Scope". It is
// entirely up to the client to populate this list with
// appropriate values; the server-performed validation does not
// (at least in this alpha) address this issue.
//
// matches. A request that specifies a target namespace matches
// only if either (a) this list contains that target namespace or
// (b) this list contains "*". Note that "*" matches any
// specified namespace but does not match a request that _does
// not specify_ a namespace (see the `clusterScope` field for
// that).
// This list may be empty, but only if `clusterScope` is true.
// +optional
// +listType=set
Namespaces []string
}

View File

@@ -245,20 +245,14 @@ func ValidateFlowSchemaResourcePolicyRule(rule *flowcontrol.ResourcePolicyRule,
allErrs = append(allErrs, field.Invalid(fldPath.Child("resources"), rule.Resources, "if '*' is present, must not specify other resources"))
}
if len(rule.Namespaces) == 0 {
allErrs = append(allErrs, field.Required(fldPath.Child("namespaces"), "resource rules must supply at least one namespace"))
} else if memberInList(flowcontrol.NamespaceEvery, rule.Namespaces...) {
for _, tgtNS := range rule.Namespaces {
if tgtNS != flowcontrol.NamespaceEvery && tgtNS != flowcontrol.NamespaceClusterScope {
allErrs = append(allErrs, field.Invalid(fldPath.Child("namespaces"), rule.Namespaces, "'*' may be accompanied only by 'Cluster Scope'"))
break
}
if len(rule.Namespaces) == 0 && !rule.ClusterScope {
allErrs = append(allErrs, field.Required(fldPath.Child("namespaces"), "resource rules that are not cluster scoped must supply at least one namespace"))
} else if hasWildcard(rule.Namespaces) {
if len(rule.Namespaces) > 1 {
allErrs = append(allErrs, field.Invalid(fldPath.Child("namespaces"), rule.Namespaces, "if '*' is present, must not specify other namespaces"))
}
} else {
for idx, tgtNS := range rule.Namespaces {
if tgtNS == flowcontrol.NamespaceClusterScope {
continue
}
for _, msg := range apimachineryvalidation.ValidateNamespaceName(tgtNS, false) {
allErrs = append(allErrs, field.Invalid(fldPath.Child("namespaces").Index(idx), tgtNS, nsErrIntro+msg))
}
@@ -268,7 +262,7 @@ func ValidateFlowSchemaResourcePolicyRule(rule *flowcontrol.ResourcePolicyRule,
return allErrs
}
const nsErrIntro = "each member of this list must be '*', 'Cluster Scope', or a DNS-1123 label; "
const nsErrIntro = "each member of this list must be '*' or a DNS-1123 label; "
// ValidateFlowSchemaStatus validates status for the flow-schema.
func ValidateFlowSchemaStatus(status *flowcontrol.FlowSchemaStatus, fldPath *field.Path) field.ErrorList {

View File

@@ -407,11 +407,11 @@ func TestFlowSchemaValidation(t *testing.T) {
},
},
expectedErrors: field.ErrorList{
field.Required(field.NewPath("spec").Child("rules").Index(0).Child("resourceRules").Index(0).Child("namespaces"), "resource rules must supply at least one namespace"),
field.Required(field.NewPath("spec").Child("rules").Index(0).Child("resourceRules").Index(0).Child("namespaces"), "resource rules that are not cluster scoped must supply at least one namespace"),
},
},
{
name: "NamespaceClusterScope is allowed",
name: "ClusterScope is allowed, with no Namespaces",
flowSchema: &flowcontrol.FlowSchema{
ObjectMeta: metav1.ObjectMeta{
Name: "system-foo",
@@ -434,7 +434,7 @@ func TestFlowSchemaValidation(t *testing.T) {
Verbs: []string{flowcontrol.VerbAll},
APIGroups: []string{flowcontrol.APIGroupAll},
Resources: []string{flowcontrol.ResourceAll},
Namespaces: []string{flowcontrol.NamespaceClusterScope},
ClusterScope: true,
},
},
},
@@ -444,7 +444,7 @@ func TestFlowSchemaValidation(t *testing.T) {
expectedErrors: field.ErrorList{},
},
{
name: "NamespaceClusterScope is allowed with NamespaceEvery",
name: "ClusterScope is allowed with NamespaceEvery",
flowSchema: &flowcontrol.FlowSchema{
ObjectMeta: metav1.ObjectMeta{
Name: "system-foo",
@@ -467,7 +467,8 @@ func TestFlowSchemaValidation(t *testing.T) {
Verbs: []string{flowcontrol.VerbAll},
APIGroups: []string{flowcontrol.APIGroupAll},
Resources: []string{flowcontrol.ResourceAll},
Namespaces: []string{flowcontrol.NamespaceClusterScope, flowcontrol.NamespaceEvery},
ClusterScope: true,
Namespaces: []string{flowcontrol.NamespaceEvery},
},
},
},
@@ -508,7 +509,7 @@ func TestFlowSchemaValidation(t *testing.T) {
},
},
expectedErrors: field.ErrorList{
field.Invalid(field.NewPath("spec").Child("rules").Index(0).Child("resourceRules").Index(0).Child("namespaces"), []string{"foo", flowcontrol.NamespaceEvery}, "'*' may be accompanied only by 'Cluster Scope'"),
field.Invalid(field.NewPath("spec").Child("rules").Index(0).Child("resourceRules").Index(0).Child("namespaces"), []string{"foo", flowcontrol.NamespaceEvery}, "if '*' is present, must not specify other namespaces"),
},
},
{

View File

@@ -29,7 +29,6 @@ const (
NameAll = "*"
NamespaceEvery = "*" // matches every particular namespace
NamespaceClusterScope = "Cluster Scope" // matches absence of namespace
)
// System preset priority level names
@@ -240,31 +239,25 @@ type ResourcePolicyRule struct {
// +listType=set
Resources []string `json:"resources" protobuf:"bytes,3,rep,name=resources"`
// `clusterScope` indicates whether to match requests that do not
// specify a namespace (which happens either because the resource
// is not namespaced or the request targets all namespaces).
// If this field is omitted or false then the `namespaces` field
// must contain a non-empty list.
// +optional
ClusterScope bool `json:"clusterScope,omitempty" protobuf:"varint,4,opt,name=clusterScope"`
// `namespaces` is a list of target namespaces that restricts
// matches. A request that does not specify a target namespace
// (which happens both when the resource is not namespaced and
// when the resource is namespaced and the request is for all
// namespaces) matches only if this list includes "Cluster Scope"
// (this string is not a valid namespace and thus can not be
// confused with an actual namespace). A request that specifies a
// target namespace matches only if either (a) this list contains
// that target namespace or (b) this list contains "*".
//
// This list may not be omitted or empty. If the list contains
// "*" then the only other allowed member is "Cluster Scope".
// Without "*", it is allowed to list "Cluster Scope" along with
// particular namespaces.
//
// Requests will match only if the values in this list are
// appropriate for the resource(s) involved. For example: for a
// cluster scoped resource (i.e., one not namespaced) a request
// can match only if this list contains "Cluster Scope". It is
// entirely up to the client to populate this list with
// appropriate values; the server-performed validation does not
// (at least in this alpha) address this issue.
//
// matches. A request that specifies a target namespace matches
// only if either (a) this list contains that target namespace or
// (b) this list contains "*". Note that "*" matches any
// specified namespace but does not match a request that _does
// not specify_ a namespace (see the `clusterScope` field for
// that).
// This list may be empty, but only if `clusterScope` is true.
// +optional
// +listType=set
Namespaces []string `json:"namespaces" protobuf:"bytes,4,rep,name=namespaces"`
Namespaces []string `json:"namespaces" protobuf:"bytes,5,rep,name=namespaces"`
}
// NonResourcePolicyRule is a predicate that matches non-resource requests according to their verb and the