UpdateStrategy RegistryPoll with nil Interval

Adds protection against a nil-pointer panic when an UpdateStrategy with nil value for RegistryPoll Interval is supplied. Also adds a unit test to ensure that the code does not panic but instead returns an error when this situation is encountered.

Signed-off-by: Daniel Franz <dfranz@redhat.com>

pkg/controller/operators/catalog/operator.go

@@ -749,16 +749,18 @@ func (o *Operator) syncRegistryServer(logger *logrus.Entry, in *v1alpha1.Catalog
  logger.Debug("ensured registry server")
 
  // requeue the catalog sync based on the polling interval, for accurate syncs of catalogs with polling enabled
- if out.Spec.UpdateStrategy != nil {
- if out.Spec.UpdateStrategy.RegistryPoll != nil {
- if out.Spec.UpdateStrategy.RegistryPoll.ParsingError != "" && out.Status.Reason != v1alpha1.CatalogSourceIntervalInvalidError {
- out.SetError(v1alpha1.CatalogSourceIntervalInvalidError, fmt.Errorf(out.Spec.UpdateStrategy.RegistryPoll.ParsingError))
- }
- logger.Debugf("requeuing registry server sync based on polling interval %s", out.Spec.UpdateStrategy.Interval.Duration.String())
- resyncPeriod := reconciler.SyncRegistryUpdateInterval(out, time.Now())
- o.catsrcQueueSet.RequeueAfter(out.GetNamespace(), out.GetName(), queueinformer.ResyncWithJitter(resyncPeriod, 0.1)())
+ if out.Spec.UpdateStrategy != nil && out.Spec.UpdateStrategy.RegistryPoll != nil {
+ if out.Spec.UpdateStrategy.Interval == nil {
+ syncError = fmt.Errorf("empty polling interval; cannot requeue registry server sync without a provided polling interval")
+ out.SetError(v1alpha1.CatalogSourceIntervalInvalidError, syncError)
  return
+ } else if out.Spec.UpdateStrategy.RegistryPoll.ParsingError != "" && out.Status.Reason != v1alpha1.CatalogSourceIntervalInvalidError {
+ out.SetError(v1alpha1.CatalogSourceIntervalInvalidError, fmt.Errorf(out.Spec.UpdateStrategy.RegistryPoll.ParsingError))
  }
+ logger.Debugf("requeuing registry server sync based on polling interval %s", out.Spec.UpdateStrategy.Interval.Duration.String())
+ resyncPeriod := reconciler.SyncRegistryUpdateInterval(out, time.Now())
+ o.catsrcQueueSet.RequeueAfter(out.GetNamespace(), out.GetName(), queueinformer.ResyncWithJitter(resyncPeriod, 0.1)())
+ return
  }
 
  if err := o.sources.Remove(sourceKey); err != nil {

pkg/controller/operators/catalog/operator_test.go

@@ -1415,6 +1415,54 @@ func TestValidateExistingCRs(t *testing.T) {
  }
 }
 
+func TestSyncRegistryServer(t *testing.T) {
+ namespace := "ns"
+
+ tests := []struct {
+ testName string
+ err error
+ catSrc *v1alpha1.CatalogSource
+ clientObjs []runtime.Object
+ }{
+ {
+ testName: "EmptyRegistryPoll",
+ err: fmt.Errorf("empty polling interval; cannot requeue registry server sync without a provided polling interval"),
+ catSrc: &v1alpha1.CatalogSource{
+ Spec: v1alpha1.CatalogSourceSpec{
+ UpdateStrategy: &v1alpha1.UpdateStrategy{
+ RegistryPoll: &v1alpha1.RegistryPoll{},
+ },
+ },
+ },
+ },
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.testName, func(t *testing.T) {
+ ctx, cancel := context.WithCancel(context.TODO())
+ defer cancel()
+
+ tt.clientObjs = append(tt.clientObjs, tt.catSrc)
+ op, err := NewFakeOperator(ctx, namespace, []string{namespace}, withClientObjs(tt.clientObjs...))
+ require.NoError(t, err)
+
+ op.reconciler = &fakes.FakeRegistryReconcilerFactory{
+ ReconcilerForSourceStub: func(source *v1alpha1.CatalogSource) reconciler.RegistryReconciler {
+ return &fakes.FakeRegistryReconciler{
+ EnsureRegistryServerStub: func(source *v1alpha1.CatalogSource) error {
+ return nil
+ },
+ }
+ },
+ }
+ require.NotPanics(t, func() {
+ _, _, err = op.syncRegistryServer(logrus.NewEntry(op.logger), tt.catSrc)
+ })
+ require.Equal(t, tt.err, err)
+ })
+ }
+}
+
 func fakeConfigMapData() map[string]string {
  data := make(map[string]string)
  yaml, err := yaml.Marshal([]apiextensionsv1beta1.CustomResourceDefinition{crd("fake-crd")})