Add more tests for ARN and its format (#19408)

Original work from #17566 modified to fit the new requirements

internal/arn/arn.go

@@ -18,6 +18,7 @@
 package arn
 
 import (
+	"errors"
 	"fmt"
 	"regexp"
 	"strings"
@@ -31,30 +32,19 @@ import (
 //
 // Reference: https://docs.aws.amazon.com/general/latest/gr/aws-arns-and-namespaces.html
 
-type arnPartition string
-
 const (
-	arnPartitionMinio arnPartition = "minio"
+	arnPrefixArn        = "arn"
-)
+	arnPartitionMinio   = "minio"
-
+	arnServiceIAM       = "iam"
-type arnService string
+	arnResourceTypeRole = "role"
-
-const (
-	arnServiceIAM arnService = "iam"
-)
-
-type arnResourceType string
-
-const (
-	arnResourceTypeRole arnResourceType = "role"
 )
 
 // ARN - representation of resources based on AWS ARNs.
 type ARN struct {
-	Partition    arnPartition
+	Partition    string
-	Service      arnService
+	Service      string
 	Region       string
-	ResourceType arnResourceType
+	ResourceType string
 	ResourceID   string
 }
 
@@ -65,7 +55,7 @@ var validResourceIDRegex = regexp.MustCompile(`[A-Za-z0-9_/\.-]+$`)
 // NewIAMRoleARN - returns an ARN for a role in MinIO.
 func NewIAMRoleARN(resourceID, serverRegion string) (ARN, error) {
 	if !validResourceIDRegex.MatchString(resourceID) {
-		return ARN{}, fmt.Errorf("Invalid resource ID: %s", resourceID)
+		return ARN{}, fmt.Errorf("invalid resource ID: %s", resourceID)
 	}
 	return ARN{
 		Partition:    arnPartitionMinio,
@@ -80,12 +70,12 @@ func NewIAMRoleARN(resourceID, serverRegion string) (ARN, error) {
 func (arn ARN) String() string {
 	return strings.Join(
 		[]string{
-			"arn",
+			arnPrefixArn,
-			string(arn.Partition),
+			arn.Partition,
-			string(arn.Service),
+			arn.Service,
 			arn.Region,
 			"", // account-id is always empty in this implementation
-			string(arn.ResourceType) + "/" + arn.ResourceID,
+			arn.ResourceType + "/" + arn.ResourceID,
 		},
 		":",
 	)
@@ -94,43 +84,41 @@ func (arn ARN) String() string {
 // Parse - parses an ARN string into a type.
 func Parse(arnStr string) (arn ARN, err error) {
 	ps := strings.Split(arnStr, ":")
-	if len(ps) != 6 ||
+	if len(ps) != 6 || ps[0] != string(arnPrefixArn) {
-		ps[0] != "arn" {
+		err = errors.New("invalid ARN string format")
-		err = fmt.Errorf("Invalid ARN string format")
 		return
 	}
 
 	if ps[1] != string(arnPartitionMinio) {
-		err = fmt.Errorf("Invalid ARN - bad partition field")
+		err = errors.New("invalid ARN - bad partition field")
 		return
 	}
 
 	if ps[2] != string(arnServiceIAM) {
-		err = fmt.Errorf("Invalid ARN - bad service field")
+		err = errors.New("invalid ARN - bad service field")
 		return
 	}
 
 	// ps[3] is region and is not validated here. If the region is invalid,
 	// the ARN would not match any configured ARNs in the server.
-
 	if ps[4] != "" {
-		err = fmt.Errorf("Invalid ARN - unsupported account-id field")
+		err = errors.New("invalid ARN - unsupported account-id field")
 		return
 	}
 
 	res := strings.SplitN(ps[5], "/", 2)
 	if len(res) != 2 {
-		err = fmt.Errorf("Invalid ARN - resource does not contain a \"/\"")
+		err = errors.New("invalid ARN - resource does not contain a \"/\"")
 		return
 	}
 
 	if res[0] != string(arnResourceTypeRole) {
-		err = fmt.Errorf("Invalid ARN: resource type is invalid.")
+		err = errors.New("invalid ARN: resource type is invalid")
 		return
 	}
 
 	if !validResourceIDRegex.MatchString(res[1]) {
-		err = fmt.Errorf("Invalid resource ID: %s", res[1])
+		err = fmt.Errorf("invalid resource ID: %s", res[1])
 		return
 	}
 

internal/arn/arn_test.go

@@ -1,6 +1,6 @@
-// Copyright (c) 2015-2023 MinIO, Inc.
+// Copyright (c) 2015-2024 MinIO, Inc.
 //
-// # This file is part of MinIO Object Storage stack
+// This file is part of MinIO Object Storage stack
 //
 // This program is free software: you can redistribute it and/or modify
 // it under the terms of the GNU Affero General Public License as published by
@@ -18,59 +18,219 @@
 package arn
 
 import (
-	"fmt"
+	"reflect"
 	"testing"
 )
 
-func TestNewIAMRoleARN(t *testing.T) {
+func TestARN_String(t *testing.T) {
-	testCases := []struct {
+	tests := []struct {
-		resourceID    string
+		arn  ARN
-		serverRegion  string
+		want string
-		expectedARN   string
-		isErrExpected bool
 	}{
 		{
-			resourceID:    "myrole",
+			arn: ARN{
-			serverRegion:  "us-east-1",
+				Partition:    "minio",
-			expectedARN:   "arn:minio:iam:us-east-1::role/myrole",
+				Service:      "iam",
-			isErrExpected: false,
+				Region:       "us-east-1",
+				ResourceType: "role",
+				ResourceID:   "my-role",
+			},
+			want: "arn:minio:iam:us-east-1::role/my-role",
 		},
 		{
-			resourceID:    "myrole",
+			arn: ARN{
-			serverRegion:  "",
+				Partition:    "minio",
-			expectedARN:   "arn:minio:iam:::role/myrole",
+				Service:      "",
-			isErrExpected: false,
+				Region:       "us-east-1",
-		},
+				ResourceType: "role",
-		{
+				ResourceID:   "my-role",
-			// Resource ID can start with a hyphen
+			},
-			resourceID:    "-myrole",
+			want: "arn:minio::us-east-1::role/my-role",
-			serverRegion:  "",
-			expectedARN:   "arn:minio:iam:::role/-myrole",
-			isErrExpected: false,
-		},
-		{
-			resourceID:    "",
-			serverRegion:  "",
-			expectedARN:   "",
-			isErrExpected: true,
 		},
 	}
-	for i, testCase := range testCases {
+	for _, tt := range tests {
-		arn, err := NewIAMRoleARN(testCase.resourceID, testCase.serverRegion)
+		t.Run(tt.want, func(t *testing.T) {
-		fmt.Println(arn, err)
+			if got := tt.arn.String(); got != tt.want {
-		if err != nil {
+				t.Errorf("ARN.String() = %v, want %v", got, tt.want)
-			if !testCase.isErrExpected {
-				t.Errorf("Test %d: Unexpected error: %v", i+1, err)
 			}
-			continue
+		})
-		}
+	}
-
+}
-		if testCase.isErrExpected {
+
-			t.Errorf("Test %d: Expected error but got none", i+1)
+func TestNewIAMRoleARN(t *testing.T) {
-		}
+	type args struct {
-		if arn.String() != testCase.expectedARN {
+		resourceID   string
-			t.Errorf("Test %d: Expected ARN %s but got %s", i+1, testCase.expectedARN, arn.String())
+		serverRegion string
-		}
+	}
-
+	tests := []struct {
+		name    string
+		args    args
+		want    ARN
+		wantErr bool
+	}{
+		{
+			name: "valid resource ID must succeed",
+			args: args{
+				resourceID:   "my-role",
+				serverRegion: "us-east-1",
+			},
+			want: ARN{
+				Partition:    "minio",
+				Service:      "iam",
+				Region:       "us-east-1",
+				ResourceType: "role",
+				ResourceID:   "my-role",
+			},
+			wantErr: false,
+		},
+		{
+			name: "valid resource ID must succeed",
+			args: args{
+				resourceID:   "-my-role",
+				serverRegion: "us-east-1",
+			},
+			want: ARN{
+				Partition:    "minio",
+				Service:      "iam",
+				Region:       "us-east-1",
+				ResourceType: "role",
+				ResourceID:   "-my-role",
+			},
+			wantErr: false,
+		},
+		{
+			name: "empty server region must succeed",
+			args: args{
+				resourceID:   "my-role",
+				serverRegion: "",
+			},
+			want: ARN{
+				Partition:    "minio",
+				Service:      "iam",
+				Region:       "",
+				ResourceType: "role",
+				ResourceID:   "my-role",
+			},
+			wantErr: false,
+		},
+		{
+			name: "empty resource ID must fail",
+			args: args{
+				resourceID:   "",
+				serverRegion: "us-east-1",
+			},
+			want:    ARN{},
+			wantErr: true,
+		},
+		{
+			name: "resource ID starting with '=' must fail",
+			args: args{
+				resourceID:   "=",
+				serverRegion: "us-east-1",
+			},
+			want:    ARN{},
+			wantErr: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := NewIAMRoleARN(tt.args.resourceID, tt.args.serverRegion)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("NewIAMRoleARN() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("NewIAMRoleARN() got = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestParse(t *testing.T) {
+	type args struct {
+		arnStr string
+	}
+	tests := []struct {
+		name    string
+		args    args
+		wantArn ARN
+		wantErr bool
+	}{
+		{
+			name: "valid ARN must succeed",
+			args: args{
+				arnStr: "arn:minio:iam:us-east-1::role/my-role",
+			},
+			wantArn: ARN{
+				Partition:    "minio",
+				Service:      "iam",
+				Region:       "us-east-1",
+				ResourceType: "role",
+				ResourceID:   "my-role",
+			},
+			wantErr: false,
+		},
+		{
+			name: "valid ARN must succeed",
+			args: args{
+				arnStr: "arn:minio:iam:us-east-1::role/-my-role",
+			},
+			wantArn: ARN{
+				Partition:    "minio",
+				Service:      "iam",
+				Region:       "us-east-1",
+				ResourceType: "role",
+				ResourceID:   "-my-role",
+			},
+			wantErr: false,
+		},
+		{
+			name: "invalid ARN length must fail",
+			args: args{
+				arnStr: "arn:minio:",
+			},
+			wantArn: ARN{},
+			wantErr: true,
+		},
+		{
+			name: "invalid ARN partition must fail",
+			args: args{
+				arnStr: "arn:invalid:iam:us-east-1::role/my-role",
+			},
+			wantArn: ARN{},
+			wantErr: true,
+		},
+		{
+			name: "invalid ARN service must fail",
+			args: args{
+				arnStr: "arn:minio:invalid:us-east-1::role/my-role",
+			},
+			wantArn: ARN{},
+			wantErr: true,
+		},
+		{
+			name: "invalid ARN resource type must fail",
+			args: args{
+				arnStr: "arn:minio:iam:us-east-1::invalid",
+			},
+			wantArn: ARN{},
+			wantErr: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			gotArn, err := Parse(tt.args.arnStr)
+			if err == nil && tt.wantErr {
+				t.Errorf("Parse() error = %v, wantErr %v", err, tt.wantErr)
+			}
+			if err != nil && !tt.wantErr {
+				t.Errorf("Parse() error = %v, wantErr %v", err, tt.wantErr)
+			}
+			if err == nil {
+				if !reflect.DeepEqual(gotArn, tt.wantArn) {
+					t.Errorf("Parse() gotArn = %v, want %v", gotArn, tt.wantArn)
+				}
+			}
+		})
 	}
 }