Merge pull request #1374 from stevendborrelli/cache-semver

Update handling of Elasticache versions

examples/cache/replicationgroup.yaml

@@ -11,12 +11,12 @@ spec:
     replicationGroupDescription: "An example replication group"
     applyModificationsImmediately: true
     engine: "redis"
-    engineVersion: "5.0.6"
+    engineVersion: "6.2"
     port: 6379
     cacheSubnetGroupNameRef:
       name: sample-cache-subnet-group
     numCacheClusters: 3
-    cacheParameterGroupName: default.redis5.0
+    cacheParameterGroupName: default.redis6.x
     cacheNodeType: cache.t3.medium
     automaticFailoverEnabled: true
   writeConnectionSecretToRef:

pkg/clients/elasticache/elasticache.go

@@ -317,17 +317,90 @@ func multiAZEnabled(maz elasticachetypes.MultiAZStatus) *bool {
 	}
 }
 
-func versionMatches(kubeVersion *string, awsVersion *string) bool {
-	switch {
-	case clients.StringValue(kubeVersion) == clients.StringValue(awsVersion):
+// PartialSemanticVersion is semantic version that does not fulfill
+// the specification. This allows for partial matching.
+type PartialSemanticVersion struct {
+	Major *int64
+	Minor *int64
+	Patch *int64
+}
+
+// ParseVersion parses the semantic version of an Elasticache Cluster
+// See https://docs.aws.amazon.com/memorydb/latest/devguide/engine-versions.html
+func ParseVersion(ver *string) (*PartialSemanticVersion, error) {
+	if ver == nil || aws.ToString(ver) == "" {
+		return nil, errors.New("empty string")
+	}
+
+	parts := strings.Split(strings.TrimSpace(aws.ToString(ver)), ".")
+
+	major, err := strconv.ParseInt(parts[0], 10, 64)
+	if err != nil {
+		return nil, errors.New("major version must be a number")
+	}
+
+	p := &PartialSemanticVersion{Major: aws.Int64(major)}
+
+	if len(parts) > 1 {
+		minor, err := strconv.ParseInt(parts[1], 10, 64)
+		// if not a digit (i.e. .x, ignore)
+		if err != nil {
+			return p, nil
+		}
+		p.Minor = aws.Int64(minor)
+	}
+
+	if len(parts) > 2 {
+		patch, err := strconv.ParseInt(parts[2], 10, 64)
+		if err != nil {
+			return p, nil
+		}
+		p.Patch = aws.Int64(patch)
+	}
+
+	return p, nil
+}
+
+// For versions before 6.x, the version string can be exact (i.e. 5.0.6)
+// For versions 6, 6.2, 6.x, etc., we only need a major version
+func versionMatches(kubeVersion *string, awsVersion *string) bool { //nolint: gocyclo
+
+	if clients.StringValue(kubeVersion) == clients.StringValue(awsVersion) {
 		return true
+	}
 
-	case kubeVersion == nil || awsVersion == nil:
+	if kubeVersion == nil || awsVersion == nil {
 		return false
+	}
 
-	default:
-		return strings.HasSuffix(*kubeVersion, ".x") && strings.HasPrefix(*awsVersion, strings.TrimSuffix(*kubeVersion, "x"))
+	kv, err := ParseVersion(kubeVersion)
+	if err != nil {
+		return false
+	}
+
+	av, err := ParseVersion(awsVersion)
+	if err != nil {
+		return false
+	}
+
+	if aws.ToInt64(kv.Major) != aws.ToInt64(av.Major) {
+		return false
 	}
+
+	if kv.Minor != nil {
+		if aws.ToInt64(kv.Minor) != aws.ToInt64(av.Minor) {
+			return false
+		}
+	}
+
+	// Setting the patch level is valid for Redis versions < 6
+	if kv.Patch != nil && aws.ToInt64(kv.Major) < 6 {
+		if aws.ToInt64(kv.Patch) != aws.ToInt64(av.Patch) {
+			return false
+		}
+	}
+
+	return true
 }
 
 func cacheClusterNeedsUpdate(kube v1beta1.ReplicationGroupParameters, cc elasticachetypes.CacheCluster) bool { // nolint:gocyclo

pkg/clients/elasticache/elasticache_test.go

@@ -23,6 +23,7 @@ import (
 
 	"github.com/aws/smithy-go/document"
 	"github.com/google/go-cmp/cmp/cmpopts"
+	"github.com/pkg/errors"
 
 	"github.com/aws/aws-sdk-go-v2/service/elasticache"
 	elasticachetypes "github.com/aws/aws-sdk-go-v2/service/elasticache/types"
@@ -1201,9 +1202,15 @@ func TestVersionMatches(t *testing.T) {
 			want:        true,
 		},
 		{
-			name:        "pattern mismatch",
-			kubeVersion: aws.String("6.x"),
-			awsVersion:  aws.String("5.0.8"),
+			name:        "minor match",
+			kubeVersion: aws.String("6.2"),
+			awsVersion:  aws.String("6.2.6"),
+			want:        true,
+		},
+		{
+			name:        "zero major mismatch",
+			kubeVersion: aws.String("0.2"),
+			awsVersion:  aws.String("6.2.6"),
 			want:        false,
 		},
 	}
@@ -1218,6 +1225,94 @@ func TestVersionMatches(t *testing.T) {
 	}
 }
 
+func TestParseVersion(t *testing.T) {
+	cases := []struct {
+		name    string
+		version *string
+		parsed  *PartialSemanticVersion
+		wantErr error
+	}{
+		{
+			name:    "nil",
+			version: nil,
+			parsed:  nil,
+			wantErr: errors.New("empty string"),
+		},
+		{
+			name:    "",
+			version: nil,
+			parsed:  nil,
+			wantErr: errors.New("empty string"),
+		},
+		{
+			name:    "bad version",
+			version: aws.String("badversion"),
+			parsed:  nil,
+			wantErr: errors.New("major version must be a number"),
+		},
+		{
+			name:    "major only",
+			version: aws.String("6"),
+			parsed:  &PartialSemanticVersion{Major: aws.Int64(6)},
+			wantErr: nil,
+		},
+		{
+			name:    "major.minor",
+			version: aws.String("6.2"),
+			parsed:  &PartialSemanticVersion{Major: aws.Int64(6), Minor: aws.Int64(2)},
+			wantErr: nil,
+		},
+		{
+			name:    "major.x",
+			version: aws.String("6.x"),
+			parsed:  &PartialSemanticVersion{Major: aws.Int64(6)},
+			wantErr: nil,
+		},
+		{
+			name:    "major.",
+			version: aws.String("6."),
+			parsed:  &PartialSemanticVersion{Major: aws.Int64(6)},
+			wantErr: nil,
+		},
+		{
+			name:    "majorLarge.",
+			version: aws.String("999."),
+			parsed:  &PartialSemanticVersion{Major: aws.Int64(999)},
+			wantErr: nil,
+		},
+		{
+			name:    "major.minor.patch",
+			version: aws.String("5.0.9"),
+			parsed:  &PartialSemanticVersion{Major: aws.Int64(5), Minor: aws.Int64(0, aws.FieldRequired), Patch: aws.Int64(9)},
+			wantErr: nil,
+		},
+		{
+			name:    "major.minor.x",
+			version: aws.String("5.0.x"),
+			parsed:  &PartialSemanticVersion{Major: aws.Int64(5), Minor: aws.Int64(0, aws.FieldRequired)},
+			wantErr: nil,
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			got, gotErr := ParseVersion(tc.version)
+			if diff := cmp.Diff(tc.parsed, got); diff != "" {
+				t.Errorf("ParseVersion(...): -want, +got:\n%s", diff)
+			}
+			if (tc.wantErr == nil) != (gotErr == nil) {
+				t.Errorf("ParseVersion Error (%+v) - got %v", tc.wantErr, gotErr)
+			}
+			if tc.wantErr != nil {
+				if tc.wantErr.Error() != gotErr.Error() {
+					t.Errorf("ParseVersion ErrorString (%s) - got %s", tc.wantErr.Error(), gotErr.Error())
+				}
+			}
+
+		})
+	}
+}
+
 func TestMultiAZEnabled(t *testing.T) {
 	f := false
 	tr := true