Skip to content

Commit

Permalink
clarify error messaging (#566)
Browse files Browse the repository at this point in the history
  • Loading branch information
katmutua authored Oct 17, 2023
1 parent 751db73 commit 31f8be8
Show file tree
Hide file tree
Showing 9 changed files with 30 additions and 31 deletions.
3 changes: 1 addition & 2 deletions .wokeignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,4 @@ dist/aks-webhooks.yaml
**/go.sum
**/go.work.sum
dist/third-party/


woke/woke.yaml
8 changes: 4 additions & 4 deletions cmd/manager/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,17 +105,17 @@ func main() {
},
})
if err != nil {
setupLog.Error(err, "unable to start manager")
setupLog.Error(err, "there was an error initializing a new `Manager` object")
os.Exit(1)
}
client, err := kubernetes.NewForConfig(mgr.GetConfig())
if err != nil {
setupLog.Error(err, "unable to create clientset")
setupLog.Error(err, "there was an error creating a new clientset using config provided")
os.Exit(1)
}
authInfoResolver, err := webhookutil.NewDefaultAuthenticationInfoResolver("")
if err != nil {
setupLog.Error(err, "unable to create authentication info resolver")
setupLog.Error(err, "there was an error creating a new authentication info resolver object")
os.Exit(1)
}
wc := binding.WebhookConfig{
Expand Down Expand Up @@ -170,7 +170,7 @@ func main() {

setupLog.Info("starting manager")
if err := mgr.Start(ctx); err != nil {
setupLog.Error(err, "problem running manager")
setupLog.Error(err, "there was an error starting the manager")
os.Exit(1)
}
}
4 changes: 2 additions & 2 deletions pkg/apis/conventions/v1alpha1/clusterpodconvention_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ func TestClusterPodConventionValidate(t *testing.T) {
},
},
expected: field.ErrorList{
field.Invalid(field.NewPath("spec", "selectorTarget"), InvalidSelectorTarget, "Accepted selector target values are \"PodIntent\" and \"PodTemplateSpec\". The default value is set to \"PodTemplateSpec\""),
field.Invalid(field.NewPath("spec", "selectorTarget"), InvalidSelectorTarget, `The value provided for the selectorTarget field is invalid. Accepted selectorTarget values include \"PodIntent\" and \"PodTemplateSpec\". The default value is set to \"PodTemplateSpec\"`),
},
},
{
Expand All @@ -305,7 +305,7 @@ func TestClusterPodConventionValidate(t *testing.T) {
},
},
expected: field.ErrorList{
field.Invalid(field.NewPath("spec", "priority"), WrongPriority, "Accepted priority values \"Early\" or \"Normal\" or \"Late\""),
field.Invalid(field.NewPath("spec", "priority"), WrongPriority, `The priority value provided is invalid. Accepted priority values include \"Early\" or \"Normal\" or \"Late\". The default value is set to \"Normal\"`),
},
}, {
name: "valid priority level",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func (s *ClusterPodConventionSpec) validate(fldPath *field.Path) field.ErrorList
}

if s.Priority != EarlyPriority && s.Priority != LatePriority && s.Priority != NormalPriority {
errs = append(errs, field.Invalid(fldPath.Child("priority"), s.Priority, "Accepted priority values \"Early\" or \"Normal\" or \"Late\""))
errs = append(errs, field.Invalid(fldPath.Child("priority"), s.Priority, `The priority value provided is invalid. Accepted priority values include \"Early\" or \"Normal\" or \"Late\". The default value is set to \"Normal\"`))
}

// Webhook will be required mutually exclusive of other options that don't exist yet
Expand All @@ -77,7 +77,7 @@ func (s *ClusterPodConventionSpec) validate(fldPath *field.Path) field.ErrorList
if s.SelectorTarget != PodTemplateSpecLabels && s.SelectorTarget != PodIntentLabels {
errs = append(errs,
field.Invalid(fldPath.Child("selectorTarget"), s.SelectorTarget,
`Accepted selector target values are "PodIntent" and "PodTemplateSpec". The default value is set to "PodTemplateSpec"`),
`The value provided for the selectorTarget field is invalid. Accepted selectorTarget values include \"PodIntent\" and \"PodTemplateSpec\". The default value is set to \"PodTemplateSpec\"`),
)
}

Expand Down
6 changes: 3 additions & 3 deletions pkg/binding/conventions.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func (c *Conventions) Filter(collectedLabels map[string]labels.Set) (Conventions
for _, selector := range selectors {
sourceLabels, err := metav1.LabelSelectorAsSelector(&selector)
if err != nil {
return nil, fmt.Errorf("converting label selector for clusterPodConvention %q failed: %v", source.Name, err)
return nil, fmt.Errorf("unable to convert label selector for ClusterPodConvention %q: %v", source.Name, err)
}

if sourceLabels.Matches(collectedLabels[string(source.SelectorTarget)]) {
Expand Down Expand Up @@ -91,7 +91,7 @@ func (c *Conventions) Apply(ctx context.Context,
) (*corev1.PodTemplateSpec, error) {
log := logr.FromContextOrDiscard(ctx)
if parent == nil {
return nil, fmt.Errorf("pod intent value cannot be nil")
return nil, fmt.Errorf("PodIntent value cannot be nil")
}
workload := parent.Spec.Template.AsPodTemplateSpec()
appliedConventions := []string{}
Expand All @@ -103,7 +103,7 @@ func (c *Conventions) Apply(ctx context.Context,
imageConfigList, err := rc.ResolveImageMetadata(ctx, workload)
if err != nil {
log.Error(err, "fetching metadata for Images failed")
return nil, fmt.Errorf("fetching metadata for Images failed: %v", err)
return nil, fmt.Errorf("failed to fetch metadata for Images: %v", err)
}
conventionRequestObj := &webhookv1alpha1.PodConventionContext{
ObjectMeta: metav1.ObjectMeta{
Expand Down
2 changes: 1 addition & 1 deletion pkg/binding/image_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ func (rc *RegistryConfig) ResolveImageMetadata(ctx context.Context, template *co
func (rc *RegistryConfig) resolveImageMetadata(ctx context.Context, imageRef string, opts ...name.Option) (webhookv1alpha1.ImageConfig, error) {
ref, resolved, err := resolveTagsToDigest(imageRef, opts...)
if err != nil {
return webhookv1alpha1.ImageConfig{}, fmt.Errorf("failed to resolve image %q: %v", imageRef, err)
return webhookv1alpha1.ImageConfig{}, fmt.Errorf("failed to resolve image %q: %v as digest could not be determined from tag provided", imageRef, err)
}
if rc.Keys == nil {
return webhookv1alpha1.ImageConfig{}, fmt.Errorf("registry config keys are not set")
Expand Down
6 changes: 3 additions & 3 deletions pkg/controllers/podintent_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ func BuildRegistryConfig(rc binding.RegistryConfig) reconcilers.SubReconciler[*c
func getCABundle(ctx context.Context, c reconcilers.Config, certRef *conventionsv1alpha1.ClusterPodConventionWebhookCertificate, parent *conventionsv1alpha1.PodIntent, convention *conventionsv1alpha1.ClusterPodConvention) ([]byte, error) {
allCertReqs := &certmanagerv1.CertificateRequestList{}
if err := c.List(ctx, allCertReqs, client.InNamespace(certRef.Namespace)); err != nil {
return nil, fmt.Errorf("failed to fetch associated certificate requests in namespace %q: %v", certRef.Namespace, err)
return nil, fmt.Errorf("failed to fetch associated `CertificateRequests` using the certificate namespace %q: %v configured on the ClusterPodConvention config", certRef.Namespace, err)
}

certReqs := []certmanagerv1.CertificateRequest{}
Expand All @@ -249,7 +249,7 @@ func getCABundle(ctx context.Context, c reconcilers.Config, certRef *conventions
}

if len(certReqs) == 0 {
return nil, fmt.Errorf("unable to find valid certificaterequests for certificate %q configured in convention %q", fmt.Sprintf("%s/%s", certRef.Namespace, certRef.Name), convention.Name)
return nil, fmt.Errorf(`unable to find valid "CertificateRequests" for certificate %q configured in convention %q`, fmt.Sprintf("%s/%s", certRef.Namespace, certRef.Name), convention.Name)
}

// take the most recent 3 certificate request CAs
Expand Down Expand Up @@ -289,7 +289,7 @@ func ApplyConventionsReconciler(wc binding.WebhookConfig) reconcilers.SubReconci
filteredAndSortedConventions, err := sources.FilterAndSort(collectedLabels)
if err != nil {
conditionManager.MarkFalse(conventionsv1alpha1.PodIntentConditionConventionsApplied, "LabelSelector", "filtering conventions failed: %v", err.Error())
log.Error(err, "failed to filter sources")
log.Error(err, "failed to filter conventions")
return ctrl.Result{}, nil
}
if workload.Annotations == nil {
Expand Down
24 changes: 12 additions & 12 deletions pkg/controllers/podintent_reconciler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -831,11 +831,11 @@ func TestResolveConventions(t *testing.T) {
dieconventionsv1alpha1.PodIntentConditionConventionsAppliedBlank.
Status(metav1.ConditionFalse).
Reason("CABundleResolutionFailed").
Message(`failed to authenticate: unable to find valid certificaterequests for certificate "ns/wrong-ca" configured in convention "test-convention"`),
Message(`failed to authenticate: unable to find valid "CertificateRequests" for certificate "ns/wrong-ca" configured in convention "test-convention"`),
dieconventionsv1alpha1.PodIntentConditionReadyBlank.
Status(metav1.ConditionFalse).
Reason("CABundleResolutionFailed").
Message(`failed to authenticate: unable to find valid certificaterequests for certificate "ns/wrong-ca" configured in convention "test-convention"`),
Message(`failed to authenticate: unable to find valid "CertificateRequests" for certificate "ns/wrong-ca" configured in convention "test-convention"`),
)
}).
DieReleasePtr(),
Expand Down Expand Up @@ -872,11 +872,11 @@ func TestResolveConventions(t *testing.T) {
dieconventionsv1alpha1.PodIntentConditionConventionsAppliedBlank.
Status(metav1.ConditionFalse).
Reason("CABundleResolutionFailed").
Message(`failed to authenticate: unable to find valid certificaterequests for certificate "test-namespace/my-cert" configured in convention "test-convention"`),
Message(`failed to authenticate: unable to find valid "CertificateRequests" for certificate "test-namespace/my-cert" configured in convention "test-convention"`),
dieconventionsv1alpha1.PodIntentConditionReadyBlank.
Status(metav1.ConditionFalse).
Reason("CABundleResolutionFailed").
Message(`failed to authenticate: unable to find valid certificaterequests for certificate "test-namespace/my-cert" configured in convention "test-convention"`),
Message(`failed to authenticate: unable to find valid "CertificateRequests" for certificate "test-namespace/my-cert" configured in convention "test-convention"`),
)
}).
DieReleasePtr(),
Expand Down Expand Up @@ -915,11 +915,11 @@ func TestResolveConventions(t *testing.T) {
dieconventionsv1alpha1.PodIntentConditionConventionsAppliedBlank.
Status(metav1.ConditionFalse).
Reason("CABundleResolutionFailed").
Message(`failed to authenticate: unable to find valid certificaterequests for certificate "test-namespace/my-cert" configured in convention "test-convention"`),
Message(`failed to authenticate: unable to find valid "CertificateRequests" for certificate "test-namespace/my-cert" configured in convention "test-convention"`),
dieconventionsv1alpha1.PodIntentConditionReadyBlank.
Status(metav1.ConditionFalse).
Reason("CABundleResolutionFailed").
Message(`failed to authenticate: unable to find valid certificaterequests for certificate "test-namespace/my-cert" configured in convention "test-convention"`),
Message(`failed to authenticate: unable to find valid "CertificateRequests" for certificate "test-namespace/my-cert" configured in convention "test-convention"`),
)
}).
DieReleasePtr(),
Expand Down Expand Up @@ -958,11 +958,11 @@ func TestResolveConventions(t *testing.T) {
dieconventionsv1alpha1.PodIntentConditionConventionsAppliedBlank.
Status(metav1.ConditionFalse).
Reason("CABundleResolutionFailed").
Message(`failed to authenticate: unable to find valid certificaterequests for certificate "test-namespace/my-cert" configured in convention "test-convention"`),
Message(`failed to authenticate: unable to find valid "CertificateRequests" for certificate "test-namespace/my-cert" configured in convention "test-convention"`),
dieconventionsv1alpha1.PodIntentConditionReadyBlank.
Status(metav1.ConditionFalse).
Reason("CABundleResolutionFailed").
Message(`failed to authenticate: unable to find valid certificaterequests for certificate "test-namespace/my-cert" configured in convention "test-convention"`),
Message(`failed to authenticate: unable to find valid "CertificateRequests" for certificate "test-namespace/my-cert" configured in convention "test-convention"`),
)
}).
DieReleasePtr(),
Expand Down Expand Up @@ -1389,11 +1389,11 @@ func TestApplyConventionsReconciler(t *testing.T) {
dieconventionsv1alpha1.PodIntentConditionConventionsAppliedBlank.
Status(metav1.ConditionFalse).
Reason("LabelSelector").
Message("filtering conventions failed: converting label selector for clusterPodConvention \"my-conventions\" failed: key: Invalid value: \"\": name part must be non-empty; name part must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName', or 'my.name', or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]')"),
Message("filtering conventions failed: unable to convert label selector for ClusterPodConvention \"my-conventions\": key: Invalid value: \"\": name part must be non-empty; name part must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName', or 'my.name', or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]')"),
dieconventionsv1alpha1.PodIntentConditionReadyBlank.
Status(metav1.ConditionFalse).
Reason("LabelSelector").
Message("filtering conventions failed: converting label selector for clusterPodConvention \"my-conventions\" failed: key: Invalid value: \"\": name part must be non-empty; name part must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName', or 'my.name', or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]')"),
Message("filtering conventions failed: unable to convert label selector for ClusterPodConvention \"my-conventions\": key: Invalid value: \"\": name part must be non-empty; name part must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName', or 'my.name', or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]')"),
)
}).
DieReleasePtr(),
Expand Down Expand Up @@ -1441,11 +1441,11 @@ func TestApplyConventionsReconciler(t *testing.T) {
dieconventionsv1alpha1.PodIntentConditionConventionsAppliedBlank.
Status(metav1.ConditionFalse).
Reason("ConventionsApplied").
Message("fetching metadata for Images failed: image: \"ubuntu\" error: registry config keys are not set"),
Message("failed to fetch metadata for Images: image: \"ubuntu\" error: registry config keys are not set"),
dieconventionsv1alpha1.PodIntentConditionReadyBlank.
Status(metav1.ConditionFalse).
Reason("ConventionsApplied").
Message("fetching metadata for Images failed: image: \"ubuntu\" error: registry config keys are not set"),
Message("failed to fetch metadata for Images: image: \"ubuntu\" error: registry config keys are not set"),
)
}).
DieReleasePtr(),
Expand Down
4 changes: 2 additions & 2 deletions webhook/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func ConventionHandler(ctx context.Context, convention Convention) func(http.Res
}
decoder := json.NewDecoder(bytes.NewBuffer(reqBody))
if derr := decoder.Decode(wc); derr != nil {
logger.Error(derr, "error decoding the request body into a PodConventionContext type")
logger.Error(derr, "the request body could not be decoded into a PodConventionContext type")
w.WriteHeader(http.StatusBadRequest)
return
}
Expand All @@ -109,7 +109,7 @@ func ConventionHandler(ctx context.Context, convention Convention) func(http.Res
wc.Status.AppliedConventions = appliedConventions
wc.Status.Template = *pts
if err := json.NewEncoder(w).Encode(wc); err != nil {
logger.Error(err, "error encoding response")
logger.Error(err, "failed to encode the PodConventionContext. Unable to create response for received request.")
return
}
}
Expand Down

0 comments on commit 31f8be8

Please sign in to comment.