This commit fixes manager component configation for

ansible-operator and helm-operator by adding the underlying reading and Manager option updating functionality. This was missing when the config file was originally added to both ansible/v1 and helm/v1 plugins. --config accepts a path to this config file. internal/{ansible,helm}: add --config flag, and configure existing flags to be applied on top of manager Options internal/cmd/{ansible,helm}: apply config file data prior to flag data Signed-off-by: Eric Stroczynski <ericstroczynski@gmail.com>

Author: estroz

changelog/fragments/add-componentconfig-flag.yaml

@@ -0,0 +1,26 @@
+entries:
+ - description: >
+ (ansible/v1, helm/v1) correctly support configuring the manager by file using the
+ `--config` flag. This flag was not added to either ansible-/helm-operator binary
+ when file support was originally added.
+ kind: bugfix
+ migration:
+ header: Add the manager config patch to config/default/kustomization.yaml
+ body: >
+ The scaffolded `--config` flag was not added to either ansible-/helm-operator binary
+ when config file support was originally added, so does not currently work.
+ The `--config` flag was added to both binaries to support configuration by file;
+ this method of configuration only applies to the underlying
+ [controller manager](https://pkg.go.dev/sigs.k8s.io/controller-runtime/pkg/manager#Manager),
+ not the operator as a whole. To configure the operator's Deployment with this config data,
+ add the following to config/default/kustomization.yaml:
+
+ ```diff
+ # If you want your controller-manager to expose the /metrics
+ # endpoint w/o any authn/z, please comment the following line.
+ - manager_auth_proxy_patch.yaml
+ +
+ +# Mount the controller config file for loading manager configurations
+ +# through a ComponentConfig type
+ +#- manager_config_patch.yaml
+ ```

internal/ansible/flags/flag.go

@@ -19,6 +19,8 @@ import (
 "time"
 
 "github.com/spf13/pflag"
+"k8s.io/client-go/tools/leaderelection/resourcelock"
+"sigs.k8s.io/controller-runtime/pkg/manager"
 )
 
 // Flags - Options to be used by an ansible operator
@@ -37,18 +39,23 @@ type Flags struct {
 LeaderElectionNamespace string
 GracefulShutdownTimeout time.Duration
 AnsibleArgs string
+
+// Path to a controller-runtime componentconfig file.
+// If this is empty, use default values.
+ManagerConfigPath string
+
+// If not nil, used to deduce which flags were set in the CLI.
+flagSet *pflag.FlagSet
 }
 
-const AnsibleRolesPathEnvVar = "ANSIBLE_ROLES_PATH"
-const AnsibleCollectionsPathEnvVar = "ANSIBLE_COLLECTIONS_PATH"
+const (
+AnsibleRolesPathEnvVar = "ANSIBLE_ROLES_PATH"
+AnsibleCollectionsPathEnvVar = "ANSIBLE_COLLECTIONS_PATH"
+)
 
 // AddTo - Add the ansible operator flags to the the flagset
 func (f *Flags) AddTo(flagSet *pflag.FlagSet) {
-flagSet.DurationVar(&f.ReconcilePeriod,
-"reconcile-period",
-time.Minute,
-"Default reconcile period for controllers",
-)
+// Ansible flags.
 flagSet.StringVar(&f.WatchesFile,
 "watches-file",
 "./watches.yaml",
@@ -59,11 +66,6 @@ func (f *Flags) AddTo(flagSet *pflag.FlagSet) {
 true,
 "The ansible operator will inject owner references unless this flag is false",
 )
-flagSet.IntVar(&f.MaxConcurrentReconciles,
-"max-concurrent-reconciles",
-runtime.NumCPU(),
-"Maximum number of concurrent reconciles for controllers. Overridden by environment variable.",
-)
 flagSet.IntVar(&f.AnsibleVerbosity,
 "ansible-verbosity",
 2,
@@ -77,9 +79,36 @@ func (f *Flags) AddTo(flagSet *pflag.FlagSet) {
 flagSet.StringVar(&f.AnsibleCollectionsPath,
 "ansible-collections-path",
 "",
-"Path to installed Ansible Collections. If set, collections should be located in {{value}}/ansible_collections/. If unset, collections are assumed to be in ~/.ansible/collections or /usr/share/ansible/collections.",
+"Path to installed Ansible Collections. If set, collections should be located in {{value}}/ansible_collections/. "+
+"If unset, collections are assumed to be in ~/.ansible/collections or /usr/share/ansible/collections.",
+)
+flagSet.StringVar(&f.AnsibleArgs,
+"ansible-args",
+"",
+"Ansible args. Allows user to specify arbitrary arguments for ansible-based operators.",
 )
-// todo:remove it for 2.0.0
+
+// Controller flags.
+flagSet.DurationVar(&f.ReconcilePeriod,
+"reconcile-period",
+time.Minute,
+"Default reconcile period for controllers",
+)
+flagSet.IntVar(&f.MaxConcurrentReconciles,
+"max-concurrent-reconciles",
+runtime.NumCPU(),
+"Maximum number of concurrent reconciles for controllers. Overridden by environment variable.",
+)
+
+// Controller manager flags.
+flagSet.StringVar(&f.ManagerConfigPath,
+"manager-config",
+"",
+"The controller will load its initial configuration from this file. "+
+"Omit this flag to use the default configuration values. "+
+"Command-line flags override configuration from this file.",
+)
+// TODO(2.0.0): remove
 flagSet.StringVar(&f.MetricsBindAddress,
 "metrics-addr",
 ":8080",
@@ -91,15 +120,14 @@ func (f *Flags) AddTo(flagSet *pflag.FlagSet) {
 ":8080",
 "The address the metric endpoint binds to",
 )
-// todo: for Go/Helm the port used is: 8081
+// TODO(2.0.0): for Go/Helm the port used is: 8081
 // update it to keep the project aligned to the other
-// types for 2.0
 flagSet.StringVar(&f.ProbeAddr,
 "health-probe-bind-address",
 ":6789",
 "The address the probe endpoint binds to.",
 )
-// todo:remove it for 2.0.0
+// TODO(2.0.0): remove
 flagSet.BoolVar(&f.LeaderElection,
 "enable-leader-election",
 false,
@@ -131,9 +159,43 @@ func (f *Flags) AddTo(flagSet *pflag.FlagSet) {
 "The amount of time that will be spent waiting"+
 " for runners to gracefully exit.",
 )
-flagSet.StringVar(&f.AnsibleArgs,
-"ansible-args",
-"",
-"Ansible args. Allows user to specify arbitrary arguments for ansible-based operators.",
-)
+}
+
+// ToManagerOptions uses the flag set in f to configure options.
+// Values of options take precedence over flag defaults,
+// as values are assume to have been explicitly set.
+func (f *Flags) ToManagerOptions(options manager.Options) manager.Options {
+// Alias FlagSet.Changed so options are still updated when fields are empty.
+changed := func(flagName string) bool {
+return f.flagSet.Changed(flagName)
+}
+if f.flagSet == nil {
+changed = func(flagName string) bool { return false }
+}
+
+// TODO(2.0.0): remove metrics-addr
+if changed("metrics-bind-address") || changed("metrics-addr") || options.MetricsBindAddress == "" {
+options.MetricsBindAddress = f.MetricsBindAddress
+}
+if changed("health-probe-bind-address") || options.HealthProbeBindAddress == "" {
+options.HealthProbeBindAddress = f.ProbeAddr
+}
+// TODO(2.0.0): remove enable-leader-election
+if changed("leader-elect") || changed("enable-leader-election") || !options.LeaderElection {
+options.LeaderElection = f.LeaderElection
+}
+if changed("leader-election-id") || options.LeaderElectionID == "" {
+options.LeaderElectionID = f.LeaderElectionID
+}
+if changed("leader-election-namespace") || options.LeaderElectionNamespace == "" {
+options.LeaderElectionNamespace = f.LeaderElectionNamespace
+}
+if options.LeaderElectionResourceLock == "" {
+options.LeaderElectionResourceLock = resourcelock.ConfigMapsResourceLock
+}
+if changed("graceful-shutdown-timeout") || options.GracefulShutdownTimeout == nil {
+options.GracefulShutdownTimeout = &f.GracefulShutdownTimeout
+}
+
+return options
 }

internal/cmd/ansible-operator/run/cmd.go

@@ -25,7 +25,7 @@ import (
 
 "github.com/spf13/cobra"
 metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-"k8s.io/client-go/tools/leaderelection/resourcelock"
+ctrl "sigs.k8s.io/controller-runtime"
 "sigs.k8s.io/controller-runtime/pkg/cache"
 "sigs.k8s.io/controller-runtime/pkg/client/config"
 "sigs.k8s.io/controller-runtime/pkg/healthz"
@@ -82,25 +82,43 @@ func run(cmd *cobra.Command, f *flags.Flags) {
 printVersion()
 metrics.RegisterBuildInfo(crmetrics.Registry)
 
+// Load config options from the config at f.ManagerConfigPath.
+// These options will not override those set by flags.
+var (
+options manager.Options
+err error
+)
+if f.ManagerConfigPath != "" {
+cfgLoader := ctrl.ConfigFile().AtPath(f.ManagerConfigPath)
+if options, err = options.AndFrom(cfgLoader); err != nil {
+log.Error(err, "Unable to load the manager config file")
+os.Exit(1)
+}
+}
+exitIfUnsupported(options)
+
 cfg, err := config.GetConfig()
 if err != nil {
 log.Error(err, "Failed to get config.")
 os.Exit(1)
 }
 
+// TODO(2.0.0): remove
 // Deprecated: OPERATOR_NAME environment variable is an artifact of the
 // legacy operator-sdk project scaffolding. Flag `--leader-election-id`
 // should be used instead.
 if operatorName, found := os.LookupEnv("OPERATOR_NAME"); found {
 log.Info("Environment variable OPERATOR_NAME has been deprecated, use --leader-election-id instead.")
-if cmd.Flags().Lookup("leader-election-id").Changed {
+if cmd.Flags().Changed("leader-election-id") {
 log.Info("Ignoring OPERATOR_NAME environment variable since --leader-election-id is set")
-} else {
-f.LeaderElectionID = operatorName
+} else if options.LeaderElectionID == "" {
+// Only set leader election ID using OPERATOR_NAME if unset everywhere else,
+// since this env var is deprecated.
+options.LeaderElectionID = operatorName
 }
 }
 
-//todo: remove the following checks for 2.0.0 they are required just because of the flags deprecation
+//TODO(2.0.0): remove the following checks. they are required just because of the flags deprecation
 if cmd.Flags().Changed("leader-elect") && cmd.Flags().Changed("enable-leader-election") {
 log.Error(errors.New("only one of --leader-elect and --enable-leader-election may be set"), "invalid flags usage")
 os.Exit(1)
@@ -113,20 +131,15 @@ func run(cmd *cobra.Command, f *flags.Flags) {
 
 // Set default manager options
 // TODO: probably should expose the host & port as an environment variables
-options := manager.Options{
-MetricsBindAddress: f.MetricsBindAddress,
-HealthProbeBindAddress: f.ProbeAddr,
-LeaderElection: f.LeaderElection,
-LeaderElectionID: f.LeaderElectionID,
-LeaderElectionResourceLock: resourcelock.ConfigMapsResourceLock,
-LeaderElectionNamespace: f.LeaderElectionNamespace,
-ClientBuilder: clientbuilder.NewUnstructedCached(),
-GracefulShutdownTimeout: &f.GracefulShutdownTimeout,
+options = f.ToManagerOptions(options)
+if options.ClientBuilder == nil {
+options.ClientBuilder = clientbuilder.NewUnstructedCached()
 }
 
 namespace, found := os.LookupEnv(k8sutil.WatchNamespaceEnvVar)
 log = log.WithValues("Namespace", namespace)
 if found {
+log.V(1).Info("Setting namespace with value in %s", k8sutil.WatchNamespaceEnvVar)
 if namespace == metav1.NamespaceAll {
 log.Info("Watching all namespaces.")
 options.Namespace = metav1.NamespaceAll
@@ -139,9 +152,9 @@ func run(cmd *cobra.Command, f *flags.Flags) {
 options.Namespace = namespace
 }
 }
-} else {
-log.Info(fmt.Sprintf("%v environment variable not set. Watching all namespaces.",
-k8sutil.WatchNamespaceEnvVar))
+} else if options.Namespace == "" {
+log.Info(fmt.Sprintf("Watch namespaces not configured by environment variable %s or file. "+
+"Watching all namespaces.", k8sutil.WatchNamespaceEnvVar))
 options.Namespace = metav1.NamespaceAll
 }
 
@@ -202,7 +215,7 @@ func run(cmd *cobra.Command, f *flags.Flags) {
 }, w.Blacklist)
 }
 
-// todo: remove when a upper version be bumped
+// TODO(2.0.0): remove
 err = mgr.AddHealthzCheck("ping", healthz.Ping)
 if err != nil {
 log.Error(err, "Failed to add Healthz check.")
@@ -219,7 +232,7 @@ func run(cmd *cobra.Command, f *flags.Flags) {
 RESTMapper: mgr.GetRESTMapper(),
 ControllerMap: cMap,
 OwnerInjection: f.InjectOwnerRef,
-WatchedNamespaces: []string{namespace},
+WatchedNamespaces: strings.Split(namespace, ","),
 })
 if err != nil {
 log.Error(err, "Error starting proxy.")
@@ -240,6 +253,27 @@ func run(cmd *cobra.Command, f *flags.Flags) {
 log.Info("Exiting.")
 }
 
+// exitIfUnsupported prints an error containing unsupported field names and exits
+// if any of those fields are not their default values.
+func exitIfUnsupported(options manager.Options) {
+var keys []string
+// The below options are webhook-specific, which is not supported by ansible.
+if options.CertDir != "" {
+keys = append(keys, "certDir")
+}
+if options.Host != "" {
+keys = append(keys, "host")
+}
+if options.Port != 0 {
+keys = append(keys, "port")
+}
+
+if len(keys) > 0 {
+log.Error(fmt.Errorf("%s set in manager options", strings.Join(keys, ", ")), "unsupported fields")
+os.Exit(1)
+}
+}
+
 // getAnsibleDebugLog return the value from the ANSIBLE_DEBUG_LOGS it order to
 // print the full Ansible logs
 func getAnsibleDebugLog() bool {