From bc839831f7563e803afb3debb7c8833e832ad3f2 Mon Sep 17 00:00:00 2001 From: AhmedGrati Date: Sat, 29 Jul 2023 19:17:08 +0100 Subject: [PATCH] test: add unit tests for the expiration date function Signed-off-by: AhmedGrati --- docs/usage/customization-guide.md | 32 +++++- source/local/local.go | 97 +++++++++++-------- source/local/local_test.go | 48 +++++++++ .../local/testdata/features.d/expired_feature | 2 + .../testdata/features.d/feature_with_comments | 2 + .../features.d/mutliple_expiration_dates | 11 +++ .../features.d/unparsable_expiry_comment | 2 + .../local/testdata/features.d/valid_feature | 2 + 8 files changed, 153 insertions(+), 43 deletions(-) create mode 100644 source/local/testdata/features.d/expired_feature create mode 100644 source/local/testdata/features.d/feature_with_comments create mode 100644 source/local/testdata/features.d/mutliple_expiration_dates create mode 100644 source/local/testdata/features.d/unparsable_expiry_comment create mode 100644 source/local/testdata/features.d/valid_feature diff --git a/docs/usage/customization-guide.md b/docs/usage/customization-guide.md index 6fe360caf8..4251fa5f05 100644 --- a/docs/usage/customization-guide.md +++ b/docs/usage/customization-guide.md @@ -316,13 +316,37 @@ Label namespace may be specified with `/[=]`. Comment lines (starting with `#`) are ignored. -You can include the following block if you want to define the expiration date -of features that are described in the feature files: +Adding following line anywhere to feature file defines date when +its content expires / is ignored: ```plaintext -# +expiry-time: 2023-07-29T11:22:33Z +# +expiry-time=2023-07-29T11:22:33Z ``` -**Note: The time format that we are supporting is RFC3339.** + +Also, the expiry-time value would stay the same during the processing of the +feature file until another expiry-time directive is encountered. +Considering the following file: + +```plaintext +# +expiry-time=2012-07-28T11:22:33Z +featureKey=featureValue + +# +expiry-time=2080-07-28T11:22:33Z +featureKey2=featureValue2 + +# +expiry-time=2070-07-28T11:22:33Z +featureKey3=featureValue3 + +# +expiry-time=2002-07-28T11:22:33Z +featureKey4=featureValue4 +``` + +After processing the above file, only `featureKey` and `featureKey3` would be +included in the list of accepted features. + +> **NOTE:** The time format that we are supporting is RFC3339. Also, the `expiry-time` +> tag is only evaluated in each re-discovery period, and the expiration of +> node labels is not tracked. ### Mounts diff --git a/source/local/local.go b/source/local/local.go index 0faeea7567..a692612ffc 100644 --- a/source/local/local.go +++ b/source/local/local.go @@ -38,9 +38,15 @@ const Name = "local" // LabelFeature of this feature source const LabelFeature = "label" -// ExpiryDateKey is the key of this feature source indicating +// ExpiryTimeKey is the key of this feature source indicating // when features should be removed. -const ExpiryDateKey = "# +expiry-time" +const ExpiryTimeKey = "expiry-time" + +// DirectivePrefix defines the prefix of directives that should be parsed +const DirectivePrefix = "# +" + +// ErrorInvalidDirectiveFormat is an error indicating that a directive has an incorrect format +var ErrorInvalidDirectiveFormat = fmt.Errorf("invalid directive format encountered while parsing features' directives. directives should have the following format: # +key=value") // Config var ( @@ -58,6 +64,11 @@ type Config struct { HooksEnabled bool `json:"hooksEnabled,omitempty"` } +// parsingOpts contains options used for directives parsing +type parsingOpts struct { + ExpiryTime time.Time +} + // Singleton source instance var ( src = localSource{config: newDefaultConfig()} @@ -149,14 +160,54 @@ func (s *localSource) GetFeatures() *nfdv1alpha1.Features { return s.features } -func parseFeatures(lines [][]byte) map[string]string { +func parseDirectives(line string, opts *parsingOpts) error { + lineSplit := strings.SplitN(line, "=", 2) + + // If we have the following directive: # +expiry-time=xxx + // we need to get the key '# +expiry-time' and then we need to extract + // the following key 'expiry-time'. + key := strings.Split(lineSplit[0], DirectivePrefix)[1] + if len(lineSplit) == 1 { + return ErrorInvalidDirectiveFormat + } + value := lineSplit[1] + + switch key { + case ExpiryTimeKey: + expiryDate, err := time.Parse(time.RFC3339, strings.TrimSpace(value)) + if err != nil { + return fmt.Errorf("failed to parse expiry-date directive: %w", err) + } + opts.ExpiryTime = expiryDate + default: + return fmt.Errorf("unknown feature file directive %q", key) + } + + return nil +} + +func parseFeatures(lines [][]byte, fileName string) map[string]string { features := make(map[string]string) + now := time.Now() + parsingOpts := &parsingOpts{ + ExpiryTime: now, + } for _, l := range lines { line := strings.TrimSpace(string(l)) if len(line) > 0 { - // Skip comment lines - if strings.HasPrefix(line, "#") { + if strings.HasPrefix(line, DirectivePrefix) { + // Parse directives + err := parseDirectives(line, parsingOpts) + if err != nil { + klog.ErrorS(err, "error while parsing directives", "fileName", fileName) + } + + continue + } + + // handle expiration + if parsingOpts.ExpiryTime.Before(now) { continue } @@ -202,7 +253,7 @@ func getFeaturesFromHooks() (map[string]string, error) { } // Append features - fileFeatures := parseFeatures(lines) + fileFeatures := parseFeatures(lines, fileName) klog.V(4).InfoS("hook executed", "fileName", fileName, "features", utils.DelayedDumper(fileFeatures)) for k, v := range fileFeatures { if old, ok := features[k]; ok { @@ -278,18 +329,7 @@ func getFeaturesFromFiles() (map[string]string, error) { } // Append features - fileFeatures := parseFeatures(lines) - - // Check expiration of file features - expiryDate, err := getExpirationDate(lines) - if err != nil { - klog.ErrorS(err, "failed to parse feature file expiry date", "fileName", fileName) - continue - } - - if expiryDate.Before(time.Now()) { - continue - } + fileFeatures := parseFeatures(lines, fileName) klog.V(4).InfoS("feature file read", "fileName", fileName, "features", utils.DelayedDumper(fileFeatures)) for k, v := range fileFeatures { @@ -303,27 +343,6 @@ func getFeaturesFromFiles() (map[string]string, error) { return features, nil } -// Return the expiration date of a feature file -func getExpirationDate(lines [][]byte) (time.Time, error) { - for _, line := range lines { - if len(line) > 0 { - lineSplit := strings.SplitN(string(line), ":", 2) - - key := lineSplit[0] - - if key == ExpiryDateKey { - expiryDate, err := time.Parse(time.RFC3339, lineSplit[1]) - if err != nil { - return time.Now(), err - } - return expiryDate, nil - } - } - } - - return time.Now(), nil -} - // Read one file func getFileContent(fileName string) ([][]byte, error) { var lines [][]byte diff --git a/source/local/local_test.go b/source/local/local_test.go index e83493c63c..0727861f8f 100644 --- a/source/local/local_test.go +++ b/source/local/local_test.go @@ -17,7 +17,10 @@ limitations under the License. package local import ( + "os" + "path/filepath" "testing" + "time" "github.com/stretchr/testify/assert" ) @@ -33,3 +36,48 @@ func TestLocalSource(t *testing.T) { assert.Empty(t, l) } + +func TestGetExpirationDate(t *testing.T) { + expectedFeaturesLen := 4 + pwd, _ := os.Getwd() + featureFilesDir = filepath.Join(pwd, "testdata/features.d") + + features, err := getFeaturesFromFiles() + + assert.NoError(t, err) + assert.Equal(t, expectedFeaturesLen, len(features)) +} + +func TestParseDirectives(t *testing.T) { + testCases := []struct { + name string + directive string + wantErr bool + }{ + { + name: "valid directive", + directive: "# +expiry-time=2080-07-28T11:22:33Z", + wantErr: false, + }, + { + name: "invalid directive", + directive: "# +random-key=random-value", + wantErr: true, + }, + { + name: "invalid directive format", + directive: "# + Something", + wantErr: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + parsingOpts := parsingOpts{ + ExpiryTime: time.Now(), + } + err := parseDirectives(tc.directive, &parsingOpts) + assert.Equal(t, err != nil, tc.wantErr) + }) + } +} diff --git a/source/local/testdata/features.d/expired_feature b/source/local/testdata/features.d/expired_feature new file mode 100644 index 0000000000..7d02b7e9b8 --- /dev/null +++ b/source/local/testdata/features.d/expired_feature @@ -0,0 +1,2 @@ +# +expiry-time=2012-07-28T11:22:33Z +featureKey=featureValue diff --git a/source/local/testdata/features.d/feature_with_comments b/source/local/testdata/features.d/feature_with_comments new file mode 100644 index 0000000000..1a45b00f66 --- /dev/null +++ b/source/local/testdata/features.d/feature_with_comments @@ -0,0 +1,2 @@ +# Notes: +featureKey=featureValue diff --git a/source/local/testdata/features.d/mutliple_expiration_dates b/source/local/testdata/features.d/mutliple_expiration_dates new file mode 100644 index 0000000000..e6fd0ba3c5 --- /dev/null +++ b/source/local/testdata/features.d/mutliple_expiration_dates @@ -0,0 +1,11 @@ +# +expiry-time=2012-07-28T11:22:33Z +featureKey=featureValue + +# +expiry-time=2080-07-28T11:22:33Z +featureKey2=featureValue2 + +# +expiry-time=2070-07-28T11:22:33Z +featureKey3=featureValue3 + +# +expiry-time=2002-07-28T11:22:33Z +featureKey4=featureValue4 diff --git a/source/local/testdata/features.d/unparsable_expiry_comment b/source/local/testdata/features.d/unparsable_expiry_comment new file mode 100644 index 0000000000..85183029b7 --- /dev/null +++ b/source/local/testdata/features.d/unparsable_expiry_comment @@ -0,0 +1,2 @@ +# +expiry-time=2080-07-28T11:22:33X +featureKey=featureValue diff --git a/source/local/testdata/features.d/valid_feature b/source/local/testdata/features.d/valid_feature new file mode 100644 index 0000000000..1e5cfe22f6 --- /dev/null +++ b/source/local/testdata/features.d/valid_feature @@ -0,0 +1,2 @@ +# +expiry-time=2080-07-28T11:22:33Z +featureKey=featureValue