test: add unit tests for the expiration date function

Signed-off-by: AhmedGrati <ahmedgrati1999@gmail.com>

docs/usage/customization-guide.md

@@ -316,13 +316,37 @@ Label namespace may be specified with `<namespace>/<name>[=<value>]`.
 
 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 `featureKey2` 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
 

source/local/local.go

@@ -38,9 +38,12 @@ 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 = "# +"
 
 // Config
 var (
@@ -58,6 +61,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 +157,56 @@ func (s *localSource) GetFeatures() *nfdv1alpha1.Features {
 	return s.features
 }
 
-func parseFeatures(lines [][]byte) map[string]string {
+func parseDirectives(line string, opts *parsingOpts) error {
+	if !strings.HasPrefix(line, DirectivePrefix) {
+		return nil
+	}
+
+	directive := line[len(DirectivePrefix):]
+	split := strings.SplitN(directive, "=", 2)
+	key := split[0]
+
+	if len(split) == 1 {
+		return fmt.Errorf("invalid directive format in %q, should be '# +key=value'", line)
+	}
+	value := split[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, "#") {
+				// 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 +252,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 +328,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 +342,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

source/local/local_test.go

@@ -17,7 +17,11 @@ limitations under the License.
 package local
 
 import (
+	"fmt"
+	"os"
+	"path/filepath"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 )
@@ -33,3 +37,48 @@ func TestLocalSource(t *testing.T) {
 	assert.Empty(t, l)
 
 }
+
+func TestGetExpirationDate(t *testing.T) {
+	expectedFeaturesLen := 5
+	pwd, _ := os.Getwd()
+	featureFilesDir = filepath.Join(pwd, "testdata/features.d")
+
+	features, err := getFeaturesFromFiles()
+	fmt.Println(features)
+	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)
+		})
+	}
+}

source/local/testdata/features.d/expired_feature

@@ -0,0 +1,2 @@
+# +expiry-time=2012-07-28T11:22:33Z
+featureKeyExpired=featureValue

source/local/testdata/features.d/feature_with_comments

@@ -0,0 +1,2 @@
+# Notes: foo bar
+featureKeyRandomComment=featureValue

source/local/testdata/features.d/multiple_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

source/local/testdata/features.d/unparsable_expiry_comment

@@ -0,0 +1,2 @@
+# +expiry-time=2080-07-28T11:22:33X
+featureKeyUnparsable=featureValue

source/local/testdata/features.d/valid_feature

@@ -0,0 +1,2 @@
+# +expiry-time=2080-07-28T11:22:33Z
+featureKeyValid=featureValue