diff --git a/internal/anchor/validator.go b/internal/anchor/validator.go index 5af038227..32aba47b9 100644 --- a/internal/anchor/validator.go +++ b/internal/anchor/validator.go @@ -97,13 +97,11 @@ func (v *Validator) handle(req *anchorRequest) admission.Response { // Can't create subnamespaces in unmanaged namespaces if why := config.WhyUnmanaged(pnm); why != "" { err := fmt.Errorf("cannot create a subnamespace in the unmanaged namespace %q (%s)", pnm, why) - // TODO(erikgb): Add to list of Invalid field errors? return webhooks.DenyForbidden(api.SubnamespaceAnchorGR, pnm, err) } // Can't create subnamespaces using unmanaged namespace names if why := config.WhyUnmanaged(cnm); why != "" { err := fmt.Errorf("cannot create a subnamespace using the unmanaged namespace name %q (%s)", cnm, why) - // TODO(erikgb): Add to list of Invalid field errors? return webhooks.DenyForbidden(api.SubnamespaceAnchorGR, cnm, err) } @@ -113,7 +111,6 @@ func (v *Validator) handle(req *anchorRequest) admission.Response { childIsMissingAnchor := (cns.Parent().Name() == pnm && cns.IsSub) if !childIsMissingAnchor { err := errors.New("cannot create a subnamespace using an existing namespace") - // TODO(erikgb): Add to list of Invalid field errors? return webhooks.DenyConflict(api.SubnamespaceAnchorGR, cnm, err) } } diff --git a/internal/hierarchyconfig/validator.go b/internal/hierarchyconfig/validator.go index 647980841..2b0b7bea4 100644 --- a/internal/hierarchyconfig/validator.go +++ b/internal/hierarchyconfig/validator.go @@ -123,12 +123,10 @@ func (v *Validator) handle(ctx context.Context, log logr.Logger, req *request) a if why := config.WhyUnmanaged(req.hc.Namespace); why != "" { err := fmt.Errorf("namespace %q is not managed by HNC (%s) and cannot be set as a child of another namespace", req.hc.Namespace, why) - // TODO(erikgb): Invalid field error better? return webhooks.DenyForbidden(api.HierarchyConfigurationGR, api.Singleton, err) } if why := config.WhyUnmanaged(req.hc.Spec.Parent); why != "" { err := fmt.Errorf("namespace %q is not managed by HNC (%s) and cannot be set as the parent of another namespace", req.hc.Spec.Parent, why) - // TODO(erikgb): Invalid field error better? return webhooks.DenyForbidden(api.HierarchyConfigurationGR, api.Singleton, err) } @@ -190,7 +188,6 @@ func (v *Validator) checkNS(ns *forest.Namespace) admission.Response { haltedRoot := ns.GetHaltedRoot() if haltedRoot != "" && haltedRoot != ns.Name() { err := fmt.Errorf("ancestor %q of namespace %q has a critical condition, which must be resolved before any changes can be made to the hierarchy configuration", haltedRoot, ns.Name()) - // TODO(erikgb): InternalError better? return webhooks.DenyForbidden(api.HierarchyConfigurationGR, api.Singleton, err) } @@ -201,7 +198,6 @@ func (v *Validator) checkNS(ns *forest.Namespace) admission.Response { func (v *Validator) checkParent(ns, curParent, newParent *forest.Namespace) admission.Response { if ns.IsExternal() && newParent != nil { err := fmt.Errorf("namespace %q is managed by %q, not HNC, so it cannot have a parent in HNC", ns.Name(), ns.Manager) - // TODO(erikgb): Invalid field error better? return webhooks.DenyForbidden(api.HierarchyConfigurationGR, api.Singleton, err) } @@ -212,14 +208,12 @@ func (v *Validator) checkParent(ns, curParent, newParent *forest.Namespace) admi // Prevent changing parent of a subnamespace if ns.IsSub { err := fmt.Errorf("illegal parent: Cannot set the parent of %q to %q because it's a subnamespace of %q", ns.Name(), newParent.Name(), curParent.Name()) - // TODO(erikgb): Invalid field error better? return webhooks.DenyConflict(api.HierarchyConfigurationGR, api.Singleton, err) } // non existence of parent namespace -> not allowed if newParent != nil && !newParent.Exists() { err := fmt.Errorf("requested parent %q does not exist", newParent.Name()) - // TODO(erikgb): Invalid field error better? return webhooks.DenyForbidden(api.HierarchyConfigurationGR, api.Singleton, err) } @@ -230,7 +224,6 @@ func (v *Validator) checkParent(ns, curParent, newParent *forest.Namespace) admi // parent conflicts with something in the _existing_ hierarchy. if reason := ns.CanSetParent(newParent); reason != "" { err := fmt.Errorf("illegal parent: %s", reason) - // TODO(erikgb): Invalid field error better? return webhooks.DenyConflict(api.HierarchyConfigurationGR, api.Singleton, err) } diff --git a/internal/hncconfig/validator.go b/internal/hncconfig/validator.go index 9b5dd38ed..60bfa0209 100644 --- a/internal/hncconfig/validator.go +++ b/internal/hncconfig/validator.go @@ -49,7 +49,6 @@ func (v *Validator) Handle(ctx context.Context, req admission.Request) admission if req.Operation == k8sadm.Delete { if req.Name == api.HNCConfigSingleton { err := errors.New("deleting the 'config' object is forbidden") - // TODO(erikgb): MethodNotSupported error better? return webhooks.DenyForbidden(api.HNCConfigurationGR, api.HNCConfigSingleton, err) } else { // We allow deleting other objects. We should never enter this case with the CRD validation. We introduced diff --git a/internal/namespace/validator.go b/internal/namespace/validator.go index e742de50d..f7999b5ef 100644 --- a/internal/namespace/validator.go +++ b/internal/namespace/validator.go @@ -136,7 +136,6 @@ func (v *Validator) illegalTreeLabel(req *nsRequest) admission.Response { // Check if new HNC label tree key isn't being added if oldLabels[key] != val { err := fmt.Errorf("cannot set or modify tree label %q in namespace %q; these can only be managed by HNC", key, req.ns.Name) - // TODO(erikgb): Invalid field error list better? return webhooks.DenyForbidden(namespaceGR, req.ns.Name, err) } } @@ -146,7 +145,6 @@ func (v *Validator) illegalTreeLabel(req *nsRequest) admission.Response { if strings.Contains(key, api.LabelTreeDepthSuffix) { if _, ok := req.ns.Labels[key]; !ok { err := fmt.Errorf("cannot remove tree label %q in namespace %q; these can only be managed by HNC", key, req.ns.Name) - // TODO(erikgb): Invalid field error list better? return webhooks.DenyForbidden(namespaceGR, req.ns.Name, err) } } @@ -175,7 +173,6 @@ func (v *Validator) illegalIncludedNamespaceLabel(req *nsRequest) admission.Resp err := fmt.Errorf("you cannot enforce webhook rules on this unmanaged namespace using the %q label. "+ "See https://github.com/kubernetes-sigs/hierarchical-namespaces/blob/master/docs/user-guide/concepts.md#included-namespace-label "+ "for detail", api.LabelIncludedNamespace) - // TODO(erikgb): Invalid field error better? return webhooks.DenyForbidden(namespaceGR, req.ns.Name, err) } @@ -187,7 +184,6 @@ func (v *Validator) illegalIncludedNamespaceLabel(req *nsRequest) admission.Resp err := fmt.Errorf("you cannot change the value of the %q label. It has to be set as true on all managed namespaces. "+ "See https://github.com/kubernetes-sigs/hierarchical-namespaces/blob/master/docs/user-guide/concepts.md#included-namespace-label "+ "for detail", api.LabelIncludedNamespace) - // TODO(erikgb): Invalid field error better? return webhooks.DenyForbidden(namespaceGR, req.ns.Name, err) } @@ -220,7 +216,6 @@ func (v *Validator) conflictBetweenParentAndExternalManager(req *nsRequest, ns * if mgr != "" && mgr != api.MetaGroup && ns.Parent() != nil { err := fmt.Errorf("is a child of %q. Namespaces with parents defined by HNC cannot also be managed externally. "+ "To manage this namespace with %q, first make it a root in HNC", ns.Parent().Name(), mgr) - // TODO(erikgb): Conflict error better? return webhooks.DenyForbidden(namespaceGR, req.ns.Name, err) } return webhooks.Allow("")