From 2228eb61cb72edd73075439eb49e20437e50c682 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Thu, 4 Apr 2024 01:31:34 -0700 Subject: [PATCH] Add more tests for ARN and its format (#19408) Original work from #17566 modified to fit the new requirements --- internal/arn/arn.go | 54 ++++----- internal/arn/arn_test.go | 252 ++++++++++++++++++++++++++++++++------- 2 files changed, 227 insertions(+), 79 deletions(-) diff --git a/internal/arn/arn.go b/internal/arn/arn.go index 291a53b33..274f294eb 100644 --- a/internal/arn/arn.go +++ b/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" -) - -type arnService string - -const ( - arnServiceIAM arnService = "iam" -) - -type arnResourceType string - -const ( - arnResourceTypeRole arnResourceType = "role" + arnPrefixArn = "arn" + arnPartitionMinio = "minio" + arnServiceIAM = "iam" + arnResourceTypeRole = "role" ) // ARN - representation of resources based on AWS ARNs. type ARN struct { - Partition arnPartition - Service arnService + Partition string + 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", - string(arn.Partition), - string(arn.Service), + arnPrefixArn, + arn.Partition, + 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 || - ps[0] != "arn" { - err = fmt.Errorf("Invalid ARN string format") + if len(ps) != 6 || ps[0] != string(arnPrefixArn) { + err = errors.New("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 } diff --git a/internal/arn/arn_test.go b/internal/arn/arn_test.go index 59dacb2d7..7012cfa1b 100644 --- a/internal/arn/arn_test.go +++ b/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) { - testCases := []struct { - resourceID string - serverRegion string - expectedARN string - isErrExpected bool +func TestARN_String(t *testing.T) { + tests := []struct { + arn ARN + want string }{ { - resourceID: "myrole", - serverRegion: "us-east-1", - expectedARN: "arn:minio:iam:us-east-1::role/myrole", - isErrExpected: false, + arn: ARN{ + Partition: "minio", + Service: "iam", + Region: "us-east-1", + ResourceType: "role", + ResourceID: "my-role", + }, + want: "arn:minio:iam:us-east-1::role/my-role", }, { - resourceID: "myrole", - serverRegion: "", - expectedARN: "arn:minio:iam:::role/myrole", - isErrExpected: false, - }, - { - // Resource ID can start with a hyphen - resourceID: "-myrole", - serverRegion: "", - expectedARN: "arn:minio:iam:::role/-myrole", - isErrExpected: false, - }, - { - resourceID: "", - serverRegion: "", - expectedARN: "", - isErrExpected: true, + arn: ARN{ + Partition: "minio", + Service: "", + Region: "us-east-1", + ResourceType: "role", + ResourceID: "my-role", + }, + want: "arn:minio::us-east-1::role/my-role", }, } - for i, testCase := range testCases { - arn, err := NewIAMRoleARN(testCase.resourceID, testCase.serverRegion) - fmt.Println(arn, err) - if err != nil { - if !testCase.isErrExpected { - t.Errorf("Test %d: Unexpected error: %v", i+1, err) + for _, tt := range tests { + t.Run(tt.want, func(t *testing.T) { + if got := tt.arn.String(); got != tt.want { + t.Errorf("ARN.String() = %v, want %v", got, tt.want) } - continue - } - - if testCase.isErrExpected { - t.Errorf("Test %d: Expected error but got none", i+1) - } - if arn.String() != testCase.expectedARN { - t.Errorf("Test %d: Expected ARN %s but got %s", i+1, testCase.expectedARN, arn.String()) - } - + }) + } +} + +func TestNewIAMRoleARN(t *testing.T) { + type args struct { + resourceID 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) + } + } + }) } }