Repository: rancher/fleet
Pull request #2216 (2024-03-10 14:35:37)

Make AlphabeticalPolicy string check case insensitive

<!-- Make sure that the referenced issue provides steps to reproduce it -->

<!-- Describe the changes introduced by this pull request -->
Previously if users wanted to use image scan with descending alphabetical policy, they had to use uppercase `DESC`, as shown below:
```yaml
imageScans:
# specify the policy to retrieve images, can be semver or alphabetical order
- policy:
    # can use ascending or descending order
    alphabetical:
      order: DESC
```
This is not very intuitive, because the [documentation](https://fleet.rancher.io/imagescan) gives an example in lowercase:
```yaml
imageScans:
# specify the policy to retrieve images, can be semver or alphabetical order
- policy:
    # if range is specified, it will take the latest image according to semver order in the range
    # for more details on how to use semver, see https://github.com/Masterminds/semver
    semver:
      range: "*"
    # can use ascending or descending order
    alphabetical:
      order: asc
```
This PR makes the string check case insensitive, removing the need to use uppercase `DESC` in the `fleet.yaml`
<!--
  Please provide a unit, integration (`./integrationtests/`) or e2e (`./e2e/`) test if possible.
-->


Patch:
diff --git a/go.mod b/go.mod
index 1a32d16f..c22d909c 100644
--- a/go.mod
+++ b/go.mod
@@ -33,7 +33,7 @@ require (
 	github.com/rancher/fleet/pkg/apis v0.0.0-00010101000000-000000000000
 	github.com/rancher/lasso v0.0.0-20230830164424-d684fdeb6f29
 	github.com/rancher/wrangler/v2 v2.1.2
-	github.com/reugn/go-quartz v0.9.0
+	github.com/reugn/go-quartz v0.11.1
 	github.com/sirupsen/logrus v1.9.3
 	github.com/spf13/cobra v1.8.0
 	github.com/stretchr/testify v1.8.4
diff --git a/go.sum b/go.sum
index 4f896b97..83844dd0 100644
--- a/go.sum
+++ b/go.sum
@@ -745,8 +745,8 @@ github.com/rancher/lasso v0.0.0-20230830164424-d684fdeb6f29 h1:+kige/h8/LnzWgPjB
 github.com/rancher/lasso v0.0.0-20230830164424-d684fdeb6f29/go.mod h1:kgk9kJVMj9FIrrXU0iyM6u/9Je4bEjPImqswkTVaKsQ=
 github.com/rancher/wrangler/v2 v2.1.2 h1:XAiReB3caOAepsTHs12AwKVv4vBJkjLosZPXbbj/85E=
 github.com/rancher/wrangler/v2 v2.1.2/go.mod h1:LgpEqT2ATL0MevUsL5rohrAEU419MIPeswsMNHw35d4=
-github.com/reugn/go-quartz v0.9.0 h1:JPaxyWx6YtNG3020DuT/LXRqishMokWwpMiLfiTWGJg=
-github.com/reugn/go-quartz v0.9.0/go.mod h1:no4ktgYbAAuY0E1SchR8cTx1LF4jYIzdgaQhzRPSkpk=
+github.com/reugn/go-quartz v0.11.1 h1:T67ZeP0h2uRxtoKFZ1Lnbf3u4jh6PBgeBlw2BUk9rW0=
+github.com/reugn/go-quartz v0.11.1/go.mod h1:no4ktgYbAAuY0E1SchR8cTx1LF4jYIzdgaQhzRPSkpk=
 github.com/rivo/uniseg v0.2.0 h1:S1pD9weZBuJdFmowNwbpi7BJ8TNftyUImj/0WQi72jY=
 github.com/rivo/uniseg v0.2.0/go.mod h1:J6wj4VEh+S6ZtnVlnTBMWIodfgj8LQOQFoIToxlJtxc=
 github.com/rogpeppe/fastuuid v1.2.0/go.mod h1:jVj6XXZzXRy/MSR5jhDC/2q6DgLz+nrA6LYCDYWNEvQ=
diff --git a/internal/cmd/controller/imagescan/gitcommit_job.go b/internal/cmd/controller/imagescan/gitcommit_job.go
index 9a7d22b8..d7677005 100644
--- a/internal/cmd/controller/imagescan/gitcommit_job.go
+++ b/internal/cmd/controller/imagescan/gitcommit_job.go
@@ -64,8 +64,8 @@ func gitCommitDescription(namespace string, name string) string {
 	return fmt.Sprintf("gitrepo-git-commit-%s-%s", namespace, name)
 }
 
-func GitCommitKey(namespace string, name string) int {
-	return quartz.HashCode(gitCommitDescription(namespace, name))
+func GitCommitKey(namespace string, name string) *quartz.JobKey {
+	return quartz.NewJobKey(gitCommitDescription(namespace, name))
 }
 
 func NewGitCommitJob(c client.Client, namespace string, name string) *GitCommitJob {
@@ -78,24 +78,22 @@ func NewGitCommitJob(c client.Client, namespace string, name string) *GitCommitJ
 	}
 }
 
-func (j *GitCommitJob) Execute(ctx context.Context) {
+func (j *GitCommitJob) Execute(ctx context.Context) error {
 	if !j.sem.TryAcquire(1) {
 		// already running
-		return
+		return nil
 	}
 	defer j.sem.Release(1)
 
 	j.cloneAndReplace(ctx)
+
+	return nil
 }
 
 func (j *GitCommitJob) Description() string {
 	return gitCommitDescription(j.namespace, j.name)
 }
 
-func (j *GitCommitJob) Key() int {
-	return GitCommitKey(j.namespace, j.name)
-}
-
 func (j *GitCommitJob) cloneAndReplace(ctx context.Context) {
 	logger := log.FromContext(ctx).WithName("imagescan-clone").WithValues("gitrepo", j.name, "namespace", j.namespace)
 	nsn := types.NamespacedName{Namespace: j.namespace, Name: j.name}
diff --git a/internal/cmd/controller/imagescan/tagscan_job.go b/internal/cmd/controller/imagescan/tagscan_job.go
index dcfc309f..9e86a605 100644
--- a/internal/cmd/controller/imagescan/tagscan_job.go
+++ b/internal/cmd/controller/imagescan/tagscan_job.go
@@ -8,6 +8,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"strings"
 	"time"
 
 	"github.com/Masterminds/semver/v3"
@@ -50,8 +51,8 @@ func tagScanDescription(namespace string, name string) string {
 	return fmt.Sprintf("image-tag-scan-%s-%s", namespace, name)
 }
 
-func TagScanKey(namespace string, name string) int {
-	return quartz.HashCode(tagScanDescription(namespace, name))
+func TagScanKey(namespace string, name string) *quartz.JobKey {
+	return quartz.NewJobKey(tagScanDescription(namespace, name))
 }
 
 func NewTagScanJob(c client.Client, namespace string, name string) *TagScanJob {
@@ -64,24 +65,22 @@ func NewTagScanJob(c client.Client, namespace string, name string) *TagScanJob {
 	}
 }
 
-func (j *TagScanJob) Execute(ctx context.Context) {
+func (j *TagScanJob) Execute(ctx context.Context) error {
 	if !j.sem.TryAcquire(1) {
 		// already running
-		return
+		return nil
 	}
 	defer j.sem.Release(1)
 
 	j.updateImageTags(ctx)
+
+	return nil
 }
 
 func (j *TagScanJob) Description() string {
 	return tagScanDescription(j.namespace, j.name)
 }
 
-func (j *TagScanJob) Key() int {
-	return TagScanKey(j.namespace, j.name)
-}
-
 func (j *TagScanJob) updateImageTags(ctx context.Context) {
 	logger := log.FromContext(ctx).WithName("imagescan-tag-scanner")
 	nsn := types.NamespacedName{Namespace: j.namespace, Name: j.name}
@@ -265,7 +264,7 @@ func latestTag(policy fleet.ImagePolicyChoice, versions []string) (string, error
 		if policy.Alphabetical.Order == "" {
 			des = true
 		} else {
-			des = policy.Alphabetical.Order == AlphabeticalOrderDesc
+			des = strings.ToUpper(policy.Alphabetical.Order) == AlphabeticalOrderDesc
 		}
 		var latest string
 		for _, version := range versions {
diff --git a/internal/cmd/controller/reconciler/imagescan_controller.go b/internal/cmd/controller/reconciler/imagescan_controller.go
index 257753f9..db80ff00 100644
--- a/internal/cmd/controller/reconciler/imagescan_controller.go
+++ b/internal/cmd/controller/reconciler/imagescan_controller.go
@@ -58,8 +58,14 @@ func (r *ImageScanReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 
 	// Make sure no duplicate jobs are scheduled. DeleteJob might return an
 	// error if the job does not exist, which we ignore.
-	_ = r.Scheduler.DeleteJob(imagescan.TagScanKey(req.Namespace, req.Name))
-	err = r.Scheduler.ScheduleJob(ctx, imagescan.NewTagScanJob(r.Client, req.Namespace, req.Name), quartz.NewSimpleTrigger(interval.Duration))
+	tagScanKey := imagescan.TagScanKey(req.Namespace, req.Name)
+	_ = r.Scheduler.DeleteJob(tagScanKey)
+	err = r.Scheduler.ScheduleJob(
+		quartz.NewJobDetail(
+			imagescan.NewTagScanJob(r.Client, req.Namespace, req.Name),
+			tagScanKey),
+		quartz.NewSimpleTrigger(interval.Duration),
+	)
 	if err != nil {
 		logger.Error(err, "Failed to schedule imagescan tagscan job")
 		return ctrl.Result{}, err
@@ -71,8 +77,14 @@ func (r *ImageScanReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
 
-	_ = r.Scheduler.DeleteJob(imagescan.GitCommitKey(gitrepo.Namespace, gitrepo.Name))
-	err = r.Scheduler.ScheduleJob(ctx, imagescan.NewGitCommitJob(r.Client, gitrepo.Namespace, gitrepo.Name), quartz.NewSimpleTrigger(interval.Duration))
+	gitCommitKey := imagescan.GitCommitKey(gitrepo.Namespace, gitrepo.Name)
+	_ = r.Scheduler.DeleteJob(gitCommitKey)
+	err = r.Scheduler.ScheduleJob(
+		quartz.NewJobDetail(
+			imagescan.NewGitCommitJob(r.Client, gitrepo.Namespace, gitrepo.Name),
+			gitCommitKey),
+		quartz.NewSimpleTrigger(interval.Duration),
+	)
 	if err != nil {
 		logger.Error(err, "Failed to schedule gitrepo gitcommit job")
 		return ctrl.Result{}, err


Test patch:
diff --git a/internal/cmd/controller/imagescan/tagscan_job_test.go b/internal/cmd/controller/imagescan/tagscan_job_test.go
new file mode 100644
index 00000000..0a3bdedb
--- /dev/null
+++ b/internal/cmd/controller/imagescan/tagscan_job_test.go
@@ -0,0 +1,58 @@
+package imagescan
+
+import (
+	"testing"
+
+	fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
+)
+
+func TestLatestTag(t *testing.T) {
+	var alphabeticalVersions = []string{"a", "b", "c"}
+
+	tests := []struct {
+		name, want string
+		policy     fleet.ImagePolicyChoice
+	}{
+		{
+			name: "alphabetical asc lowercase",
+			policy: fleet.ImagePolicyChoice{
+				Alphabetical: &fleet.AlphabeticalPolicy{Order: "asc"},
+			},
+			want: "a",
+		},
+		{
+			name: "alphabetical asc uppercase",
+			policy: fleet.ImagePolicyChoice{
+				Alphabetical: &fleet.AlphabeticalPolicy{Order: "ASC"},
+			},
+			want: "a",
+		},
+		{
+			name: "alphabetical desc lowercase",
+			policy: fleet.ImagePolicyChoice{
+				Alphabetical: &fleet.AlphabeticalPolicy{Order: "desc"},
+			},
+			want: "c",
+		},
+		{
+			name: "alphabetical desc uppercase",
+			policy: fleet.ImagePolicyChoice{
+				Alphabetical: &fleet.AlphabeticalPolicy{Order: "DESC"},
+			},
+			want: "c",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := latestTag(tt.policy, alphabeticalVersions)
+			if err != nil {
+				t.Fatalf("Error calling latestTag: %v", err)
+			}
+
+			if got != tt.want {
+				t.Errorf("latestTag() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}