Add resync period functionality for TrustRoot resource (sigstore#1458)

* add resync period for trustroot resource

Signed-off-by: Meredith Lancaster <malancas@github.com>

* remove unused struct

Signed-off-by: Meredith Lancaster <malancas@github.com>

* add new logging content

Signed-off-by: Meredith Lancaster <malancas@github.com>

* use flag.Duration

Signed-off-by: Meredith Lancaster <malancas@github.com>

* drop custom logging search now that parsing is handled by flag

Signed-off-by: Meredith Lancaster <malancas@github.com>

* drop beginning of error string

Signed-off-by: Meredith Lancaster <malancas@github.com>

* remove unneeded trustroot resync test

Signed-off-by: Meredith Lancaster <malancas@github.com>

* remove now unneeded test

Signed-off-by: Meredith Lancaster <malancas@github.com>

---------

Signed-off-by: Meredith Lancaster <malancas@github.com>

Author: malancas

.github/workflows/kind-cluster-image-policy-resync-period.yaml

@@ -97,7 +97,11 @@ var (
 
 	// policyResyncPeriod holds the interval which ClusterImagePolicies will resync
 	// This is essential for triggering a reconcile update for potentially stale KMS authorities.
-	policyResyncPeriod = flag.String("policy-resync-period", "10h", "The resync period for ClusterImagePolicies. The default is 10h.")
+	policyResyncPeriod = flag.Duration("policy-resync-period", 10*time.Hour, "The resync period for ClusterImagePolicies. The default is 10h.")
+
+	// trustrootResyncPeriod holds the interval which the TrustRoot will resync
+	// This is essential for triggering a reconcile update for potentially stale TUF metadata.
+	trustrootResyncPeriod = flag.Duration("trustroot-resync-period", 24*time.Hour, "The resync period for ClusterImagePolicies. The default is 24h.")
 )
 
 func main() {
@@ -130,11 +134,9 @@ func main() {
 		}
 	}
 
-	if duration, err := time.ParseDuration(*policyResyncPeriod); err != nil {
-		logging.FromContext(ctx).Panicf("Failed to parse --policy-resync-period '%s' : %v", *policyResyncPeriod, err)
-	} else {
-		ctx = clusterimagepolicy.ToContext(ctx, duration)
-	}
+	// Set the policy and trust root resync periods
+	ctx = clusterimagepolicy.ToContext(ctx, *policyResyncPeriod)
+	ctx = trustroot.ToContext(ctx, *trustrootResyncPeriod)
 
 	// This must match the set of resources we configure in
 	// cmd/webhook/main.go in the "types" map.

cmd/webhook/main.go

@@ -16,6 +16,7 @@ package trustroot
 
 import (
 	"context"
+	"time"
 
 	"k8s.io/client-go/tools/cache"
 	kubeclient "knative.dev/pkg/client/injection/kube/client"
@@ -36,6 +37,8 @@ import (
 // use it in tests as well.
 const FinalizerName = "trustroots.policy.sigstore.dev"
 
+type trustrootResyncPeriodKey struct{}
+
 // NewController creates a Reconciler and returns the result of NewImpl.
 func NewController(
 	ctx context.Context,
@@ -63,20 +66,34 @@ func NewController(
 	// ConfigMap but there are no changes to the TrustRoot, it needs
 	// to be synced.
 	grCb := func(obj interface{}) {
-		logging.FromContext(ctx).Info("Doing a global resync on TrustRoot due to ConfigMap changing.")
+		logging.FromContext(ctx).Info("Doing a global resync on TrustRoot due to ConfigMap changing or resync period.")
 		impl.GlobalResync(trustrootInformer.Informer())
 	}
 	// Resync on only ConfigMap changes that pertain to the one I care about.
 	// We could also fetch/construct the store and use CM watcher for it, but
 	// since we need a lister for it anyways in the reconciler, just set up
 	// the watch here.
-	if _, err := configMapInformer.Informer().AddEventHandler(cache.FilteringResourceEventHandler{
+	if _, err := configMapInformer.Informer().AddEventHandlerWithResyncPeriod(cache.FilteringResourceEventHandler{
 		FilterFunc: pkgreconciler.ChainFilterFuncs(
 			pkgreconciler.NamespaceFilterFunc(system.Namespace()),
 			pkgreconciler.NameFilterFunc(config.SigstoreKeysConfigName)),
 		Handler: controller.HandleAll(grCb),
-	}); err != nil {
-		logging.FromContext(ctx).Warnf("Failed configMapInformer AddEventHandler() %v", err)
+	}, FromContextOrDefaults(ctx)); err != nil {
+		logging.FromContext(ctx).Warnf("Failed configMapInformer AddEventHandlerWithResyncPeriod() %v", err)
 	}
 	return impl
 }
+
+func ToContext(ctx context.Context, duration time.Duration) context.Context {
+	return context.WithValue(ctx, trustrootResyncPeriodKey{}, duration)
+}
+
+// FromContextOrDefaults returns a stored trustrootResyncPeriod if attached.
+// If not found, it returns a default duration
+func FromContextOrDefaults(ctx context.Context) time.Duration {
+	x, ok := ctx.Value(trustrootResyncPeriodKey{}).(time.Duration)
+	if ok {
+		return x
+	}
+	return controller.DefaultResyncPeriod
+}

pkg/reconciler/trustroot/controller.go

@@ -16,8 +16,10 @@ package trustroot
 
 import (
 	"testing"
+	"time"
 
 	"knative.dev/pkg/configmap"
+	"knative.dev/pkg/controller"
 	rtesting "knative.dev/pkg/reconciler/testing"
 
 	// Fake injection informers
@@ -37,3 +39,21 @@ func TestNew(t *testing.T) {
 		t.Fatal("Expected NewController to return a non-nil value")
 	}
 }
+
+func TestContextDuration(t *testing.T) {
+	ctx, _ := rtesting.SetupFakeContext(t)
+
+	expected := controller.DefaultResyncPeriod
+	actual := FromContextOrDefaults(ctx)
+	if expected != actual {
+		t.Fatal("Expected the context to store the value and be retrievable")
+	}
+
+	expected = time.Hour
+	ctx = ToContext(ctx, expected)
+	actual = FromContextOrDefaults(ctx)
+
+	if expected != actual {
+		t.Fatal("Expected the context to store the value and be retrievable")
+	}
+}