Update static validator to return multiple validation issues at the s…

…ame time where applicable for better UX

pkg/api/admin/platformworkloadidentityroleset.go

@@ -33,14 +33,14 @@ type PlatformWorkloadIdentityRoleSetProperties struct {
 // PlatformWorkloadIdentityRole represents a mapping from a particular OCP operator to the built-in role that should be assigned to that operator's corresponding managed identity.
 type PlatformWorkloadIdentityRole struct {
 	// OperatorName represents the name of the operator that this role is for.
-	OperatorName string `json:"operatorName,omitempty" mutable:"true"`
+	OperatorName string `json:"operatorName,omitempty" mutable:"true" validate:"required"`
 
 	// RoleDefinitionName represents the name of the role.
-	RoleDefinitionName string `json:"roleDefinitionName,omitempty" mutable:"true"`
+	RoleDefinitionName string `json:"roleDefinitionName,omitempty" mutable:"true" validate:"required"`
 
 	// RoleDefinitionID represents the resource ID of the role definition.
-	RoleDefinitionID string `json:"roleDefinitionId,omitempty" mutable:"true"`
+	RoleDefinitionID string `json:"roleDefinitionId,omitempty" mutable:"true" validate:"required"`
 
 	// ServiceAccounts represents the set of service accounts associated with the given operator, since each service account needs its own federated credential.
-	ServiceAccounts []string `json:"serviceAccounts,omitempty" mutable:"true"`
+	ServiceAccounts []string `json:"serviceAccounts,omitempty" mutable:"true" validate:"required"`
 }

pkg/api/admin/platformworkloadidentityroleset_validatestatic.go

@@ -3,6 +3,7 @@ package admin
 import (
 	"fmt"
 	"net/http"
+	"strings"
 
 	"github.com/Azure/ARO-RP/pkg/api"
 	"github.com/Azure/ARO-RP/pkg/api/util/immutable"
@@ -42,24 +43,30 @@ func (sv platformWorkloadIdentityRoleSetStaticValidator) validate(new *PlatformW
 		return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, "properties.platformWorkloadIdentityRoles", "Must be provided and must be non-empty")
 	}
 
+	missingProperties := []string{}
+
 	for i, r := range new.Properties.PlatformWorkloadIdentityRoles {
 		if r.OperatorName == "" {
-			return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, fmt.Sprintf("properties.platformWorkloadIdentityRoles[%d].operatorName", i), "Must be provided")
+			missingProperties = append(missingProperties, fmt.Sprintf("properties.platformWorkloadIdentityRoles[%d].operatorName", i))
 		}
 
 		if r.RoleDefinitionName == "" {
-			return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, fmt.Sprintf("properties.platformWorkloadIdentityRoles[%d].roleDefinitionName", i), "Must be provided")
+			missingProperties = append(missingProperties, fmt.Sprintf("properties.platformWorkloadIdentityRoles[%d].roleDefinitionName", i))
 		}
 
 		if r.RoleDefinitionID == "" {
-			return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, fmt.Sprintf("properties.platformWorkloadIdentityRoles[%d].roleDefinitionId", i), "Must be provided")
+			missingProperties = append(missingProperties, fmt.Sprintf("properties.platformWorkloadIdentityRoles[%d].roleDefinitionId", i))
 		}
 
 		if r.ServiceAccounts == nil || len(r.ServiceAccounts) == 0 {
-			return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, fmt.Sprintf("properties.platformWorkloadIdentityRoles[%d].serviceAccounts", i), "Must be provided and must be non-empty")
+			missingProperties = append(missingProperties, fmt.Sprintf("properties.platformWorkloadIdentityRoles[%d].serviceAccounts", i))
 		}
 	}
 
+	if len(missingProperties) > 0 {
+		return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, strings.Join(missingProperties, ", "), "Must be provided")
+	}
+
 	return nil
 }
 

pkg/frontend/admin_platformworkloadidentityroleset_put_test.go

@@ -643,7 +643,7 @@ func TestPlatformWorkloadIdentityRoleSetPut(t *testing.T) {
 				},
 			},
 			wantStatusCode: http.StatusBadRequest,
-			wantError:      "400: InvalidParameter: properties.platformWorkloadIdentityRoles[0].serviceAccounts: Must be provided and must be non-empty",
+			wantError:      "400: InvalidParameter: properties.platformWorkloadIdentityRoles[0].serviceAccounts: Must be provided",
 			wantDocuments: []*api.PlatformWorkloadIdentityRoleSetDocument{
 				{
 					ID: "08080808-0808-0808-0808-080808080001",
@@ -674,7 +674,7 @@ func TestPlatformWorkloadIdentityRoleSetPut(t *testing.T) {
 			},
 		},
 		{
-			name: "updating known version requires non-empty PlatformWorkloadIdentityRole.ServiceAccounts",
+			name: "updating known version requires PlatformWorkloadIdentityRole.RoleDefinitionId and PlatformWorkloadIdentityRole.ServiceAccounts (tests the case where multiple attributes are missing and error message consists of missing properties joined together)",
 			fixture: func(f *testdatabase.Fixture) {
 				f.AddPlatformWorkloadIdentityRoleSetDocuments(
 					&api.PlatformWorkloadIdentityRoleSetDocument{
@@ -711,14 +711,12 @@ func TestPlatformWorkloadIdentityRoleSetPut(t *testing.T) {
 						{
 							OperatorName:       "ClusterIngressOperator",
 							RoleDefinitionName: "Azure RedHat OpenShift Cluster Ingress Operator Role",
-							RoleDefinitionID:   "/providers/Microsoft.Authorization/roleDefinitions/a1f96423-95ce-4224-ab27-4e3dc72facd4",
-							ServiceAccounts:    []string{},
 						},
 					},
 				},
 			},
 			wantStatusCode: http.StatusBadRequest,
-			wantError:      "400: InvalidParameter: properties.platformWorkloadIdentityRoles[0].serviceAccounts: Must be provided and must be non-empty",
+			wantError:      "400: InvalidParameter: properties.platformWorkloadIdentityRoles[0].roleDefinitionId, properties.platformWorkloadIdentityRoles[0].serviceAccounts: Must be provided",
 			wantDocuments: []*api.PlatformWorkloadIdentityRoleSetDocument{
 				{
 					ID: "08080808-0808-0808-0808-080808080001",