Validate resource names for drift remediation flags (#117)

[fixes aws-controllers-k8s/community#1647]

As discussed in #106

this patch brings resource name validation to the arguments passed to
`--reconcile-resource-resync-seconds`. It also slightly changes the
previously implemented `ParseReconcileResourceResyncSeconds` to avoid
uncessary validation ops.

Signed-off-by: Amine Hilaly <[email protected]>

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Author: a-hilaly

pkg/config/config.go

@@ -27,11 +27,13 @@ import (
 	"github.com/jaypipes/envutil"
 	flag "github.com/spf13/pflag"
 	"go.uber.org/zap/zapcore"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	ctrlrt "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 
 	ackv1alpha1 "github.com/aws-controllers-k8s/runtime/apis/core/v1alpha1"
 	acktags "github.com/aws-controllers-k8s/runtime/pkg/tags"
+	ackutil "github.com/aws-controllers-k8s/runtime/pkg/util"
 )
 
 const (
@@ -211,7 +213,15 @@ func (cfg *Config) SetAWSAccountID() error {
 }
 
 // Validate ensures the options are valid
-func (cfg *Config) Validate() error {
+func (cfg *Config) Validate(options ...Option) error {
+	merged := mergeOptions(options)
+	if len(merged.gvks) > 0 {
+		err := cfg.validateReconcileConfigResources(merged.gvks)
+		if err != nil {
+			return fmt.Errorf("invalid value for flag '%s': %v", flagReconcileResourceResyncSeconds, err)
+		}
+	}
+
 	if cfg.Region == "" {
 		return errors.New("unable to start service controller as AWS region is missing. Please pass --aws-region flag or set AWS_REGION environment variable")
 	}
@@ -257,12 +267,6 @@ func (cfg *Config) Validate() error {
 	if cfg.ReconcileDefaultResyncSeconds < 0 {
 		return fmt.Errorf("invalid value for flag '%s': resync seconds default must be greater than 0", flagReconcileDefaultResyncSeconds)
 	}
-
-	_, err := cfg.ParseReconcileResourceResyncSeconds()
-	if err != nil {
-		return fmt.Errorf("invalid value for flag '%s': %v", flagReconcileResourceResyncSeconds, err)
-	}
-
 	return nil
 }
 
@@ -275,6 +279,28 @@ func (cfg *Config) checkUnsafeEndpoint(endpoint *url.URL) error {
 	return nil
 }
 
+// validateReconcileConfigResources validates the --reconcile-resource-resync-seconds flag
+// by checking the resource names and their corresponding duration.
+func (cfg *Config) validateReconcileConfigResources(supportedGVKs []schema.GroupVersionKind) error {
+	validResourceNames := []string{}
+	for _, gvk := range supportedGVKs {
+		validResourceNames = append(validResourceNames, gvk.Kind)
+	}
+	for _, resourceResyncSecondsFlag := range cfg.ReconcileResourceResyncSeconds {
+		resourceName, _, err := parseReconcileFlagArgument(resourceResyncSecondsFlag)
+		if err != nil {
+			return fmt.Errorf("error parsing flag argument '%v': %v. Expected format: resource=seconds", resourceResyncSecondsFlag, err)
+		}
+		if !ackutil.InStrings(resourceName, validResourceNames) {
+			return fmt.Errorf(
+				"error parsing flag argument '%v': resource '%v' is not managed by this controller. Expected one of %v",
+				resourceResyncSecondsFlag, resourceName, strings.Join(validResourceNames, ", "),
+			)
+		}
+	}
+	return nil
+}
+
 // ParseReconcileResourceResyncSeconds parses the values of the --reconcile-resource-resync-seconds
 // flag and returns a map that maps resource names to resync periods.
 // The flag arguments are expected to have the format "resource=seconds", where "resource" is the
@@ -284,10 +310,7 @@ func (cfg *Config) ParseReconcileResourceResyncSeconds() (map[string]time.Durati
 	resourceResyncPeriods := make(map[string]time.Duration, len(cfg.ReconcileResourceResyncSeconds))
 	for _, resourceResyncSecondsFlag := range cfg.ReconcileResourceResyncSeconds {
 		// Parse the resource name and resync period from the flag argument
-		resourceName, resyncSeconds, err := parseReconcileFlagArgument(resourceResyncSecondsFlag)
-		if err != nil {
-			return nil, fmt.Errorf("error parsing flag argument '%v': %v. Expected format: resource=seconds", resourceResyncSecondsFlag, err)
-		}
+		resourceName, resyncSeconds, _ := parseReconcileFlagArgument(resourceResyncSecondsFlag)
 		resourceResyncPeriods[strings.ToLower(resourceName)] = time.Duration(resyncSeconds)
 	}
 	return resourceResyncPeriods, nil

pkg/config/option.go

@@ -0,0 +1,45 @@
+// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License"). You may
+// not use this file except in compliance with the License. A copy of the
+// License is located at
+//
+//     http://aws.amazon.com/apache2.0/
+//
+// or in the "license" file accompanying this file. This file is distributed
+// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+// express or implied. See the License for the specific language governing
+// permissions and limitations under the License.
+
+package config
+
+import (
+	"k8s.io/apimachinery/pkg/runtime/schema"
+)
+
+// Option is a struct used to help validating the controller
+// configuration
+type Option struct {
+	// gvks is an array containing the resources gvks (Camel-cased names)
+	// managed by the controller
+	gvks []schema.GroupVersionKind
+}
+
+// WithGVKs instructs the configuration to validate against a set of
+// supplied resource kinds and their respective groups.
+func WithGVKs(gvks []schema.GroupVersionKind) Option {
+	return Option{gvks: gvks}
+}
+
+// mergeOptions merges all Option structs into a single Option
+// and sets any defaults to missing values
+func mergeOptions(opts []Option) Option {
+	merged := Option{}
+	for _, opt := range opts {
+		if len(opt.gvks) > 0 {
+			merged.gvks = opt.gvks
+		}
+	}
+	// TODO: set some default values...
+	return merged
+}