Skip to content

Commit

Permalink
Change StatusReason for invalid requests from StatusReasonForbidden t…
Browse files Browse the repository at this point in the history
…o StatusReasonInvalid

This PR changes the StatusReason from StatusReasonForbidden to StatusReasonInvalid when handling invalid requests in `hncconfig.go`. To show the custom error message when the request is invalid, we need to add the message in the `Status.Details` field.

Issue: kubernetes-retired#411
  • Loading branch information
sophieliu15 committed Mar 20, 2020
1 parent 34e1da9 commit 248a5da
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 15 deletions.
35 changes: 27 additions & 8 deletions incubator/hnc/pkg/validators/hierarchy.go
Original file line number Diff line number Diff line change
Expand Up @@ -428,14 +428,33 @@ func allow(msg string) admission.Response {
// human-readable message _and_ a machine-readable reason, and also sets the code correctly instead
// of hardcoding it to 403 Forbidden.
func deny(reason metav1.StatusReason, msg string) admission.Response {
return admission.Response{AdmissionResponse: admissionv1beta1.AdmissionResponse{
Allowed: false,
Result: &metav1.Status{
Code: codeFromReason(reason),
Message: msg,
Reason: reason,
},
}}
if reason != metav1.StatusReasonInvalid {
return admission.Response{AdmissionResponse: admissionv1beta1.AdmissionResponse{
Allowed: false,
Result: &metav1.Status{
Code: codeFromReason(reason),
Message: msg,
Reason: reason,
},
}}
} else {
// metav1.StatusReasonInvalid shows the custom message in the Details field instead of
// Message field of metav1.Status.
return admission.Response{AdmissionResponse: admissionv1beta1.AdmissionResponse{
Allowed: false,
Result: &metav1.Status{
Code: codeFromReason(reason),
Reason: reason,
Details: &metav1.StatusDetails{
Causes: []metav1.StatusCause{
{
Message: msg,
},
},
},
},
}}
}
}

// codeFromReason implements the needed subset of
Expand Down
14 changes: 7 additions & 7 deletions incubator/hnc/pkg/validators/hncconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func (c *HNCConfig) Handle(ctx context.Context, req admission.Request) admission
// allowing it to be more easily unit tested (ie without constructing a full admission.Request).
func (c *HNCConfig) handle(ctx context.Context, inst *api.HNCConfiguration) admission.Response {
if inst.GetName() != "config" {
return deny(metav1.StatusReasonForbidden, fmt.Sprintf("Wrong singleton name: %s; the name should be 'config'", inst.GetName()))
return deny(metav1.StatusReasonInvalid, fmt.Sprintf("Wrong singleton name: %s; the name should be 'config'", inst.GetName()))
}

roleExist := false
Expand Down Expand Up @@ -102,10 +102,10 @@ func (c *HNCConfig) handle(ctx context.Context, inst *api.HNCConfiguration) admi
}
}
if !roleExist {
return deny(metav1.StatusReasonForbidden, "Configuration for Role is missing")
return deny(metav1.StatusReasonInvalid, "Configuration for Role is missing")
}
if !roleBindingExist {
return deny(metav1.StatusReasonForbidden, "Configuration for RoleBinding is missing")
return deny(metav1.StatusReasonInvalid, "Configuration for RoleBinding is missing")
}
return allow("")
}
Expand All @@ -114,7 +114,7 @@ func (c *HNCConfig) handle(ctx context.Context, inst *api.HNCConfiguration) admi
func (c *HNCConfig) isTypeConfigured(t api.TypeSynchronizationSpec, ts gvkSet) admission.Response {
gvk := schema.FromAPIVersionAndKind(t.APIVersion, t.Kind)
if exists := ts[gvk]; exists {
return deny(metav1.StatusReasonForbidden, fmt.Sprintf("Duplicate configurations for %s", gvk))
return deny(metav1.StatusReasonInvalid, fmt.Sprintf("Duplicate configurations for %s", gvk))
}
ts[gvk] = true
return allow("")
Expand All @@ -129,7 +129,7 @@ func (c *HNCConfig) validateRBAC(mode api.SynchronizationMode, kind string) admi
if mode == api.Propagate || mode == "" {
return allow("")
}
return deny(metav1.StatusReasonForbidden, fmt.Sprintf("Invalid mode of %s; current mode: %s; expected mode %s", kind, mode, api.Propagate))
return deny(metav1.StatusReasonInvalid, fmt.Sprintf("Invalid mode of %s; current mode: %s; expected mode %s", kind, mode, api.Propagate))
}

// validateType validates a non-RBAC type.
Expand All @@ -138,7 +138,7 @@ func (c *HNCConfig) validateType(ctx context.Context, t api.TypeSynchronizationS

// Validate if the GVK exists in the apiserver.
if err := c.validator.Exists(ctx, gvk); err != nil {
return deny(metav1.StatusReasonForbidden,
return deny(metav1.StatusReasonInvalid,
fmt.Sprintf("Cannot find the %s in the apiserver with error: %s", gvk, err.Error()))
}

Expand All @@ -147,7 +147,7 @@ func (c *HNCConfig) validateType(ctx context.Context, t api.TypeSynchronizationS
case api.Propagate, api.Ignore, api.Remove, "":
return allow("")
default:
return deny(metav1.StatusReasonForbidden, fmt.Sprintf("Unrecognized mode '%s' for %s", t.Mode, gvk))
return deny(metav1.StatusReasonInvalid, fmt.Sprintf("Unrecognized mode '%s' for %s", t.Mode, gvk))
}
}

Expand Down

0 comments on commit 248a5da

Please sign in to comment.