Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 704ec6b

Browse files
committedMar 6, 2023
Validate resource names for drift remediation flags
As discussed in aws-controllers-k8s#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]>
1 parent a8bd391 commit 704ec6b

File tree

2 files changed

+79
-11
lines changed

2 files changed

+79
-11
lines changed
 

Diff for: ‎pkg/config/config.go

+34-11
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,13 @@ import (
2727
"github.com/jaypipes/envutil"
2828
flag "github.com/spf13/pflag"
2929
"go.uber.org/zap/zapcore"
30+
"k8s.io/apimachinery/pkg/runtime/schema"
3031
ctrlrt "sigs.k8s.io/controller-runtime"
3132
"sigs.k8s.io/controller-runtime/pkg/log/zap"
3233

3334
ackv1alpha1 "github.com/aws-controllers-k8s/runtime/apis/core/v1alpha1"
3435
acktags "github.com/aws-controllers-k8s/runtime/pkg/tags"
36+
ackutil "github.com/aws-controllers-k8s/runtime/pkg/util"
3537
)
3638

3739
const (
@@ -211,7 +213,15 @@ func (cfg *Config) SetAWSAccountID() error {
211213
}
212214

213215
// Validate ensures the options are valid
214-
func (cfg *Config) Validate() error {
216+
func (cfg *Config) Validate(options ...Option) error {
217+
merged := mergeOptions(options)
218+
if len(merged.gks) > 0 {
219+
err := cfg.validateReconcileConfigResources(merged.gks)
220+
if err != nil {
221+
return fmt.Errorf("invalid value for flag '%s': %v", flagReconcileResourceResyncSeconds, err)
222+
}
223+
}
224+
215225
if cfg.Region == "" {
216226
return errors.New("unable to start service controller as AWS region is missing. Please pass --aws-region flag or set AWS_REGION environment variable")
217227
}
@@ -257,12 +267,6 @@ func (cfg *Config) Validate() error {
257267
if cfg.ReconcileDefaultResyncSeconds < 0 {
258268
return fmt.Errorf("invalid value for flag '%s': resync seconds default must be greater than 0", flagReconcileDefaultResyncSeconds)
259269
}
260-
261-
_, err := cfg.ParseReconcileResourceResyncSeconds()
262-
if err != nil {
263-
return fmt.Errorf("invalid value for flag '%s': %v", flagReconcileResourceResyncSeconds, err)
264-
}
265-
266270
return nil
267271
}
268272

@@ -275,6 +279,28 @@ func (cfg *Config) checkUnsafeEndpoint(endpoint *url.URL) error {
275279
return nil
276280
}
277281

282+
// validateReconcileConfigResources validates the --reconcile-resource-resync-seconds flag
283+
// by checking the resource names and their corresponding duration.
284+
func (cfg *Config) validateReconcileConfigResources(supportedGVKs []*schema.GroupKind) error {
285+
validResourceNames := []string{}
286+
for _, gvk := range supportedGVKs {
287+
validResourceNames = append(validResourceNames, gvk.Kind)
288+
}
289+
for _, resourceResyncSecondsFlag := range cfg.ReconcileResourceResyncSeconds {
290+
resourceName, _, err := parseReconcileFlagArgument(resourceResyncSecondsFlag)
291+
if err != nil {
292+
return fmt.Errorf("error parsing flag argument '%v': %v. Expected format: resource=seconds", resourceResyncSecondsFlag, err)
293+
}
294+
if !ackutil.InStrings(resourceName, validResourceNames) {
295+
return fmt.Errorf(
296+
"error parsing flag argument '%v': resource '%v' is not managed by this controller. Expected one of %v",
297+
resourceResyncSecondsFlag, resourceName, strings.Join(validResourceNames, ", "),
298+
)
299+
}
300+
}
301+
return nil
302+
}
303+
278304
// ParseReconcileResourceResyncSeconds parses the values of the --reconcile-resource-resync-seconds
279305
// flag and returns a map that maps resource names to resync periods.
280306
// 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
284310
resourceResyncPeriods := make(map[string]time.Duration, len(cfg.ReconcileResourceResyncSeconds))
285311
for _, resourceResyncSecondsFlag := range cfg.ReconcileResourceResyncSeconds {
286312
// Parse the resource name and resync period from the flag argument
287-
resourceName, resyncSeconds, err := parseReconcileFlagArgument(resourceResyncSecondsFlag)
288-
if err != nil {
289-
return nil, fmt.Errorf("error parsing flag argument '%v': %v. Expected format: resource=seconds", resourceResyncSecondsFlag, err)
290-
}
313+
resourceName, resyncSeconds, _ := parseReconcileFlagArgument(resourceResyncSecondsFlag)
291314
resourceResyncPeriods[strings.ToLower(resourceName)] = time.Duration(resyncSeconds)
292315
}
293316
return resourceResyncPeriods, nil

Diff for: ‎pkg/config/option.go

+45
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License"). You may
4+
// not use this file except in compliance with the License. A copy of the
5+
// License is located at
6+
//
7+
// http://aws.amazon.com/apache2.0/
8+
//
9+
// or in the "license" file accompanying this file. This file is distributed
10+
// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
11+
// express or implied. See the License for the specific language governing
12+
// permissions and limitations under the License.
13+
14+
package config
15+
16+
import (
17+
"k8s.io/apimachinery/pkg/runtime/schema"
18+
)
19+
20+
// Option is a struct used to help validating the controller
21+
// configuration
22+
type Option struct {
23+
// gvks is an array containing the resources gvks (Camel-cased names)
24+
// managed by the controller
25+
gks []*schema.GroupKind
26+
}
27+
28+
// WithGroupKinds instructs the configuration to validate against a set of
29+
// supplied resource kinds and their respective groups.
30+
func WithGroupKinds(gks []*schema.GroupKind) Option {
31+
return Option{gks: gks}
32+
}
33+
34+
// mergeOptions merges all Option structs into a single Option
35+
// and sets any defaults to missing values
36+
func mergeOptions(opts []Option) Option {
37+
merged := Option{}
38+
for _, opt := range opts {
39+
if len(opt.gks) > 0 {
40+
merged.gks = opt.gks
41+
}
42+
}
43+
// TODO: set some default values...
44+
return merged
45+
}

0 commit comments

Comments
 (0)
Please sign in to comment.