Skip to content

Commit

Permalink
Watch status subresource updates to trigger reconcile (#364)
Browse files Browse the repository at this point in the history
ServiceBindings setup a validating webhook to watch for changes on the
service resource. This was only watching for requests on the main
resource and did not observe the status subresource. This meant that if
a provisioned service rotated the secret name and made no other changes,
the new secret would not be observed until the regular reconcile of the
ServiceBinding (up to 10 hours later).

This change watches the status subresource for tracked resources.

Signed-off-by: Scott Andrews <andrewssc@vmware.com>
  • Loading branch information
scothis committed Nov 27, 2023
1 parent 54017e1 commit 0973d3d
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 11 deletions.
12 changes: 9 additions & 3 deletions controllers/webhook_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package controllers

import (
"context"
"fmt"
"reflect"

"github.com/go-logr/logr"
Expand Down Expand Up @@ -60,7 +61,7 @@ func AdmissionProjectorReconciler(c reconcilers.Config, name string, accessCheck
Reconciler: reconcilers.Sequence[client.Object]{
LoadServiceBindings(req),
InterceptGVKs(),
WebhookRules([]admissionregistrationv1.OperationType{admissionregistrationv1.Create, admissionregistrationv1.Update}, accessChecker),
WebhookRules([]admissionregistrationv1.OperationType{admissionregistrationv1.Create, admissionregistrationv1.Update}, []string{}, accessChecker),
},
},
DesiredResource: func(ctx context.Context, resource *admissionregistrationv1.MutatingWebhookConfiguration) (*admissionregistrationv1.MutatingWebhookConfiguration, error) {
Expand Down Expand Up @@ -194,7 +195,7 @@ func TriggerReconciler(c reconcilers.Config, name string, accessChecker rbac.Acc
LoadServiceBindings(req),
TriggerGVKs(),
InterceptGVKs(),
WebhookRules([]admissionregistrationv1.OperationType{admissionregistrationv1.Create, admissionregistrationv1.Update, admissionregistrationv1.Delete}, accessChecker),
WebhookRules([]admissionregistrationv1.OperationType{admissionregistrationv1.Create, admissionregistrationv1.Update, admissionregistrationv1.Delete}, []string{"status"}, accessChecker),
},
},
DesiredResource: func(ctx context.Context, resource *admissionregistrationv1.ValidatingWebhookConfiguration) (*admissionregistrationv1.ValidatingWebhookConfiguration, error) {
Expand Down Expand Up @@ -332,7 +333,7 @@ func TriggerGVKs() reconcilers.SubReconciler[client.Object] {
}
}

func WebhookRules(operations []admissionregistrationv1.OperationType, accessChecker rbac.AccessChecker) reconcilers.SubReconciler[client.Object] {
func WebhookRules(operations []admissionregistrationv1.OperationType, subresources []string, accessChecker rbac.AccessChecker) reconcilers.SubReconciler[client.Object] {
return &reconcilers.SyncReconciler[client.Object]{
Name: "WebhookRules",
Sync: func(ctx context.Context, _ client.Object) error {
Expand Down Expand Up @@ -377,6 +378,11 @@ func WebhookRules(operations []admissionregistrationv1.OperationType, accessChec
if resources.Len() == 0 {
continue
}
for _, resource := range resources.List() {
for _, subresource := range subresources {
resources.Insert(fmt.Sprintf("%s/%s", resource, subresource))
}
}

rules = append(rules, admissionregistrationv1.RuleWithOperations{
Operations: operations,
Expand Down
16 changes: 8 additions & 8 deletions controllers/webhook_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,7 @@ func TestTriggerReconciler(t *testing.T) {
dieadmissionregistrationv1.RuleWithOperationsBlank.
APIGroups("apps").
APIVersions("*").
Resources("deployments").
Resources("deployments", "deployments/status").
Operations(
admissionregistrationv1.Create,
admissionregistrationv1.Update,
Expand All @@ -556,7 +556,7 @@ func TestTriggerReconciler(t *testing.T) {
dieadmissionregistrationv1.RuleWithOperationsBlank.
APIGroups("example").
APIVersions("*").
Resources("myservices").
Resources("myservices", "myservices/status").
Operations(
admissionregistrationv1.Create,
admissionregistrationv1.Update,
Expand Down Expand Up @@ -1010,7 +1010,7 @@ func TestWebhookRules(t *testing.T) {
Rule: admissionregistrationv1.Rule{
APIGroups: []string{"apps"},
APIVersions: []string{"*"},
Resources: []string{"deployments"},
Resources: []string{"deployments", "deployments/status"},
},
},
},
Expand All @@ -1037,7 +1037,7 @@ func TestWebhookRules(t *testing.T) {
Rule: admissionregistrationv1.Rule{
APIGroups: []string{"apps"},
APIVersions: []string{"*"},
Resources: []string{"deployments"},
Resources: []string{"deployments", "deployments/status"},
},
},
},
Expand Down Expand Up @@ -1065,7 +1065,7 @@ func TestWebhookRules(t *testing.T) {
Rule: admissionregistrationv1.Rule{
APIGroups: []string{"apps"},
APIVersions: []string{"*"},
Resources: []string{"deployments", "statefulsets"},
Resources: []string{"deployments", "deployments/status", "statefulsets", "statefulsets/status"},
},
},
},
Expand Down Expand Up @@ -1094,15 +1094,15 @@ func TestWebhookRules(t *testing.T) {
Rule: admissionregistrationv1.Rule{
APIGroups: []string{"apps"},
APIVersions: []string{"*"},
Resources: []string{"deployments"},
Resources: []string{"deployments", "deployments/status"},
},
},
{
Operations: operations,
Rule: admissionregistrationv1.Rule{
APIGroups: []string{"batch"},
APIVersions: []string{"*"},
Resources: []string{"jobs"},
Resources: []string{"jobs", "jobs/status"},
},
},
},
Expand Down Expand Up @@ -1161,7 +1161,7 @@ func TestWebhookRules(t *testing.T) {
restMapper.Add(schema.GroupVersionKind{Group: "apps", Version: "v1", Kind: "StatefulSet"}, meta.RESTScopeNamespace)
restMapper.Add(schema.GroupVersionKind{Group: "batch", Version: "v1", Kind: "Job"}, meta.RESTScopeNamespace)
accessChecker := rbac.NewAccessChecker(c, 0).WithVerb("get")
return controllers.WebhookRules(operations, accessChecker)
return controllers.WebhookRules(operations, []string{"status"}, accessChecker)
})
}

Expand Down

0 comments on commit 0973d3d

Please sign in to comment.