Add status to ClusterImagePolicy + TrustRoot CRDs #533
Conversation
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## main #533 +/- ##
==========================================
- Coverage 54.79% 54.41% -0.38%
==========================================
Files 42 45 +3
Lines 4559 4723 +164
==========================================
+ Hits 2498 2570 +72
- Misses 1856 1949 +93
+ Partials 205 204 -1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
ko failed to install on the test failure: |
| @@ -90,28 +97,40 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, cip *v1alpha1.ClusterIma | |||
| if err != nil { | |||
| if !apierrs.IsNotFound(err) { | |||
| logging.FromContext(ctx).Errorf("Failed to get configmap: %v", err) | |||
| cip.Status.MarkCMUpdateFailed(err.Error()) | |||
There was a problem hiding this comment.
Should we add a status for the url's failure or success operation when fetching a policy ?
There was a problem hiding this comment.
We already do on line 82 (if inlinePolicies fail) we mark that failed and the error will be reflected in the status just like if any inline policy fails. Splitting out whether it's CM or URL makes things way more complicated and I see no benefit to the user for it.
There was a problem hiding this comment.
I thought we wanted to give more descriptive errors about what it could go wrong with a policy.
|
|
||
| ClusterImagePolicyConditionKeysInlined apis.ConditionType = "KeysInlined" | ||
| ClusterImagePolicyConditionPoliciesInlined apis.ConditionType = "PoliciesInlined" | ||
| ClusterImagePolicyConditionCMUpdated apis.ConditionType = "ConfigMapUpdated" |
There was a problem hiding this comment.
Do we want to add a new status to register any issue when deleting a cip https://github.com/sigstore/policy-controller/pull/533/files#diff-993c349b6a7980667de0868f0e1c1fb43da04c80bafb462c89249ab478a76a61R155 ?
There was a problem hiding this comment.
Hm. That link doesn't take me anywhere except the full diff :) Assuming you mean FinalizeKind, once the finalizer has been slapped on, I am fairly certain you can't update the object anymore. But, I could be wrong. I'll look at doing that in a followup, ok?
There was a problem hiding this comment.
Sure. FinalizeKind is optional, and if implemented by the reconciler will be called when the resource's deletion timestamp is set I am not sure what means to set the deletion timestmap.
Signed-off-by: Ville Aikas <vaikas@chainguard.dev>
Signed-off-by: Ville Aikas <vaikas@chainguard.dev>
Signed-off-by: Ville Aikas <vaikas@chainguard.dev>
Signed-off-by: Ville Aikas <vaikas@chainguard.dev>
Summary
Add Status to CRDs. Also add some more failure testing and ensure they show up in status.
Fixes: #456
Release Note
🎁 Add Status to ClusterImagePolicy as well as TrustRoot CRDs.
Documentation