diff --git a/pkg/resources/management.cattle.io/v3/project/validator.go b/pkg/resources/management.cattle.io/v3/project/validator.go index a9c6b919b..a7b9d855a 100644 --- a/pkg/resources/management.cattle.io/v3/project/validator.go +++ b/pkg/resources/management.cattle.io/v3/project/validator.go @@ -165,7 +165,7 @@ func (a *admitter) admitCommonCreateUpdate(oldProject, newProject *v3.Project) ( if fieldErr != nil { return admission.ResponseBadRequest(fieldErr.Error()), nil } - fieldErr, err = a.checkQuotaValues(&nsQuota.Limit, &projectQuota.Limit, oldProject) + fieldErr, err = a.checkQuotaValues(&nsQuota.Limit, &projectQuota.Limit, newProject) if err != nil { return nil, fmt.Errorf("error checking quota values: %w", err) } @@ -258,20 +258,24 @@ func checkQuotaFields(projectQuota *v3.ProjectResourceQuota, nsQuota *v3.Namespa return nil, nil } -func (a *admitter) checkQuotaValues(nsQuota, projectQuota *v3.ResourceQuotaLimit, oldProject *v3.Project) (*field.Error, error) { +// checkQuotaValues ensures that the new quotas are consistent with the new +// used-limit. checking against the old used-limit is contra-indicated as it +// may contain bogus data, causing failure to validate. making it impossible to +// replace a bogus used-limit with good values. +func (a *admitter) checkQuotaValues(nsQuota, projectQuota *v3.ResourceQuotaLimit, newProject *v3.Project) (*field.Error, error) { // check quota on new project fieldErr, err := namespaceQuotaFits(nsQuota, projectQuota) if err != nil || fieldErr != nil { return fieldErr, err } - // if there is no old project or no quota on the old project, no further validation needed - if oldProject == nil || oldProject.Spec.ResourceQuota == nil { + // if there is no new project or no quota on the new project, no further validation needed + if newProject == nil || newProject.Spec.ResourceQuota == nil { return nil, nil } // check quota relative to used quota - return usedQuotaFits(&oldProject.Spec.ResourceQuota.UsedLimit, projectQuota) + return usedQuotaFits(&newProject.Spec.ResourceQuota.UsedLimit, projectQuota) } func namespaceQuotaFits(namespaceQuota, projectQuota *v3.ResourceQuotaLimit) (*field.Error, error) { diff --git a/pkg/resources/management.cattle.io/v3/project/validator_test.go b/pkg/resources/management.cattle.io/v3/project/validator_test.go index 7d942e42e..4c44b7437 100644 --- a/pkg/resources/management.cattle.io/v3/project/validator_test.go +++ b/pkg/resources/management.cattle.io/v3/project/validator_test.go @@ -990,7 +990,7 @@ func TestProjectValidation(t *testing.T) { wantAllowed: false, }, { - name: "update with project quota less than used quota", + name: "update with project quota less than used quota, quota changed", operation: admissionv1.Update, oldProject: &v3.Project{ ObjectMeta: metav1.ObjectMeta{ @@ -1038,6 +1038,154 @@ func TestProjectValidation(t *testing.T) { }, wantAllowed: false, }, + { + name: "update with project quota less than used quota, used quota changed", + operation: admissionv1.Update, + oldProject: &v3.Project{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "testcluster", + }, + Spec: v3.ProjectSpec{ + ClusterName: "testcluster", + ResourceQuota: &v3.ProjectResourceQuota{ + Limit: v3.ResourceQuotaLimit{ + ConfigMaps: "100", + }, + UsedLimit: v3.ResourceQuotaLimit{ + ConfigMaps: "100", + }, + }, + NamespaceDefaultResourceQuota: &v3.NamespaceResourceQuota{ + Limit: v3.ResourceQuotaLimit{ + ConfigMaps: "50", + }, + }, + }, + }, + newProject: &v3.Project{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "testcluster", + }, + Spec: v3.ProjectSpec{ + ClusterName: "testcluster", + ResourceQuota: &v3.ProjectResourceQuota{ + Limit: v3.ResourceQuotaLimit{ + ConfigMaps: "100", + }, + UsedLimit: v3.ResourceQuotaLimit{ + ConfigMaps: "1000", + }, + }, + NamespaceDefaultResourceQuota: &v3.NamespaceResourceQuota{ + Limit: v3.ResourceQuotaLimit{ + ConfigMaps: "50", + }, + }, + }, + }, + wantAllowed: false, + }, + { + name: "update project with bogus used quota, to good used quota", + operation: admissionv1.Update, + oldProject: &v3.Project{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "testcluster", + }, + Spec: v3.ProjectSpec{ + ClusterName: "testcluster", + ResourceQuota: &v3.ProjectResourceQuota{ + Limit: v3.ResourceQuotaLimit{ + ConfigMaps: "100", + }, + UsedLimit: v3.ResourceQuotaLimit{ + ConfigMaps: "xxxxxxxxxx", + }, + }, + NamespaceDefaultResourceQuota: &v3.NamespaceResourceQuota{ + Limit: v3.ResourceQuotaLimit{ + ConfigMaps: "50", + }, + }, + }, + }, + newProject: &v3.Project{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "testcluster", + }, + Spec: v3.ProjectSpec{ + ClusterName: "testcluster", + ResourceQuota: &v3.ProjectResourceQuota{ + Limit: v3.ResourceQuotaLimit{ + ConfigMaps: "100", + }, + UsedLimit: v3.ResourceQuotaLimit{ + ConfigMaps: "100", + }, + }, + NamespaceDefaultResourceQuota: &v3.NamespaceResourceQuota{ + Limit: v3.ResourceQuotaLimit{ + ConfigMaps: "50", + }, + }, + }, + }, + wantAllowed: true, + }, + { + name: "update project with good used quota, to bogus used quota", + operation: admissionv1.Update, + oldProject: &v3.Project{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "testcluster", + }, + Spec: v3.ProjectSpec{ + ClusterName: "testcluster", + ResourceQuota: &v3.ProjectResourceQuota{ + Limit: v3.ResourceQuotaLimit{ + ConfigMaps: "100", + }, + UsedLimit: v3.ResourceQuotaLimit{ + ConfigMaps: "100", + }, + }, + NamespaceDefaultResourceQuota: &v3.NamespaceResourceQuota{ + Limit: v3.ResourceQuotaLimit{ + ConfigMaps: "50", + }, + }, + }, + }, + newProject: &v3.Project{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "testcluster", + }, + Spec: v3.ProjectSpec{ + ClusterName: "testcluster", + ResourceQuota: &v3.ProjectResourceQuota{ + Limit: v3.ResourceQuotaLimit{ + ConfigMaps: "100", + }, + UsedLimit: v3.ResourceQuotaLimit{ + ConfigMaps: "xxxxxxxxxx", + }, + }, + NamespaceDefaultResourceQuota: &v3.NamespaceResourceQuota{ + Limit: v3.ResourceQuotaLimit{ + ConfigMaps: "50", + }, + }, + }, + }, + wantErr: true, + wantAllowed: false, + }, { name: "update with fields changed in project quota less than used quota", operation: admissionv1.Update,