add logic to differentiate between 404 errors and the other. Add test…

… case to capture the flow.

pkg/cli/cmd/resourcetype/create/create.go

@@ -18,8 +18,10 @@ package create
 
 import (
 	"context"
+	"strings"
 
 	"github.com/radius-project/radius/pkg/cli"
+	"github.com/radius-project/radius/pkg/cli/clients"
 	"github.com/radius-project/radius/pkg/cli/clierrors"
 	"github.com/radius-project/radius/pkg/cli/cmd/commonflags"
 	"github.com/radius-project/radius/pkg/cli/cmd/resourcetype/common"
@@ -34,6 +36,10 @@ import (
 	aztoken "github.com/radius-project/radius/pkg/azure/tokencredentials"
 )
 
+const (
+	fakeServerResourceProviderNotFoundResponse = "unexpected status code 404. acceptable values are http.StatusOK"
+)
+
 // NewCommand creates an instance of the `rad resource-type create` command and runner.
 func NewCommand(factory framework.Factory) (*cobra.Command, framework.Runner) {
 	runner := NewRunner(factory)
@@ -136,12 +142,16 @@ func (r *Runner) Run(ctx context.Context) error {
 		}
 	}
 
-	// If resource provider does not exist or if there is any other error, first register the resource provider.
 	response, err := r.UCPClientFactory.NewResourceProvidersClient().Get(ctx, "local", r.ResourceProvider.Name, nil)
 	if err != nil {
-		r.Output.LogInfo("Resource provider %q not found.", r.ResourceProvider.Name)
-		if registerErr := manifest.RegisterFile(ctx, r.UCPClientFactory, "local", r.ResourceProviderManifestFilePath, r.Logger); err != nil {
-			return registerErr
+		// The second clause is required for testing purpose since fake server returns a different type of error.
+		if clients.Is404Error(err) || strings.Contains(err.Error(), fakeServerResourceProviderNotFoundResponse) {
+			r.Output.LogInfo("Resource provider %q not found.", r.ResourceProvider.Name)
+			if registerErr := manifest.RegisterFile(ctx, r.UCPClientFactory, "local", r.ResourceProviderManifestFilePath, r.Logger); err != nil {
+				return registerErr
+			}
+		} else {
+			return err
 		}
 	} else {
 		r.Output.LogInfo("Resource provider %q found. Registering resource type %q.", r.ResourceProvider.Name, r.ResourceTypeName)

pkg/cli/cmd/resourcetype/create/create_test.go

@@ -94,7 +94,6 @@ func Test_Run(t *testing.T) {
 		require.NotContains(t, logOutput, fmt.Sprintf("Creating resource provider %s", runner.ResourceProvider.Name))
 		require.Contains(t, logOutput, fmt.Sprintf("Resource type %s/%s created successfully", resourceProviderData.Name, expectedResourceType))
 	})
-	// another test for failure
 	t.Run("Resource provider does not exist", func(t *testing.T) {
 		resourceProviderData, err := manifest.ReadFile("testdata/valid.yaml")
 		require.NoError(t, err)
@@ -125,5 +124,33 @@ func Test_Run(t *testing.T) {
 		require.Contains(t, logOutput, fmt.Sprintf("Creating resource provider %s", runner.ResourceProvider.Name))
 
 	})
+	t.Run("Get Resource provider Internal Error", func(t *testing.T) {
+		resourceProviderData, err := manifest.ReadFile("testdata/valid.yaml")
+		require.NoError(t, err)
+
+		expectedResourceType := "testResources"
+
+		clientFactory, err := manifest.NewTestClientFactory(manifest.WithResourceProviderServerInternalError)
+		require.NoError(t, err)
+
+		var logBuffer bytes.Buffer
+		logger := func(format string, args ...any) {
+			fmt.Fprintf(&logBuffer, format+"\n", args...)
+		}
+
+		runner := &Runner{
+			UCPClientFactory:                 clientFactory,
+			Output:                           &output.MockOutput{},
+			Workspace:                        &workspaces.Workspace{},
+			ResourceProvider:                 resourceProviderData,
+			Format:                           "table",
+			Logger:                           logger,
+			ResourceProviderManifestFilePath: "testdata/valid.yaml",
+			ResourceTypeName:                 expectedResourceType,
+		}
 
+		err = runner.Run(context.Background())
+		require.Error(t, err)
+		require.Contains(t, err.Error(), "Internal Error")
+	})
 }

pkg/cli/manifest/registermanifest.go

@@ -29,12 +29,10 @@ import (
 
 // RegisterFile registers a manifest file
 func RegisterFile(ctx context.Context, clientFactory *v20231001preview.ClientFactory, planeName string, filePath string, logger func(format string, args ...any)) error {
-	// Check for valid file path
 	if filePath == "" {
 		return fmt.Errorf("invalid manifest file path")
 	}
 
-	// Read the manifest file
 	resourceProvider, err := ReadFile(filePath)
 	if err != nil {
 		return err
@@ -124,7 +122,6 @@ func RegisterFile(ctx context.Context, clientFactory *v20231001preview.ClientFac
 
 // RegisterDirectory registers all manifest files in a directory
 func RegisterDirectory(ctx context.Context, clientFactory *v20231001preview.ClientFactory, planeName string, directoryPath string, logger func(format string, args ...any)) error {
-	// Check for valid directory path
 	if directoryPath == "" {
 		return fmt.Errorf("invalid manifest directory")
 	}
@@ -138,16 +135,14 @@ func RegisterDirectory(ctx context.Context, clientFactory *v20231001preview.Clie
 		return fmt.Errorf("manifest path %s is not a directory", directoryPath)
 	}
 
-	// List all files in the manifestDirectory
 	files, err := os.ReadDir(directoryPath)
 	if err != nil {
 		return err
 	}
 
-	// Iterate over each file in the directory
 	for _, fileInfo := range files {
 		if fileInfo.IsDir() {
-			continue // Skip directories
+			continue
 		}
 		filePath := filepath.Join(directoryPath, fileInfo.Name())
 
@@ -163,18 +158,15 @@ func RegisterDirectory(ctx context.Context, clientFactory *v20231001preview.Clie
 
 // RegisterType registers a type specified in a manifest file
 func RegisterType(ctx context.Context, clientFactory *v20231001preview.ClientFactory, planeName string, filePath string, typeName string, logger func(format string, args ...any)) error {
-	// Check for valid file path
 	if filePath == "" {
 		return fmt.Errorf("invalid manifest file path")
 	}
 
-	// Read the manifest file
 	resourceProvider, err := ReadFile(filePath)
 	if err != nil {
 		return err
 	}
 
-	// Check if the type exists in the manifest file
 	resourceType, ok := resourceProvider.Types[typeName]
 	if !ok {
 		return fmt.Errorf("Type %s not found in manifest file %s", typeName, filePath)
@@ -203,7 +195,7 @@ func RegisterType(ctx context.Context, clientFactory *v20231001preview.ClientFac
 
 	var defaultAPIVersion string
 	if resourceType.DefaultAPIVersion == nil {
-		defaultAPIVersion = "2023-10-01-preview"
+		defaultAPIVersion = "2023-10-01-preview" //hardcoded for now, since we don't have a default API version in the manifest but it should be made mandatory as part of schema validation
 	} else {
 		defaultAPIVersion = *resourceType.DefaultAPIVersion
 	}

pkg/cli/manifest/testclientfactory.go

@@ -228,3 +228,39 @@ func WithResourceProviderServerNotFoundError() ucpfake.ResourceProvidersServer {
 	}
 	return resourceProvidersNotFoundServer
 }
+
+func WithResourceProviderServerInternalError() ucpfake.ResourceProvidersServer {
+	resourceProvidersServerInternalError := ucpfake.ResourceProvidersServer{
+		BeginCreateOrUpdate: func(
+			ctx context.Context,
+			planeName string,
+			resourceProviderName string,
+			resource v20231001preview.ResourceProviderResource,
+			options *v20231001preview.ResourceProvidersClientBeginCreateOrUpdateOptions,
+		) (resp azfake.PollerResponder[v20231001preview.ResourceProvidersClientCreateOrUpdateResponse], errResp azfake.ErrorResponder) {
+			// Simulate successful creation
+			result := v20231001preview.ResourceProvidersClientCreateOrUpdateResponse{
+				ResourceProviderResource: resource,
+			}
+			resp.AddNonTerminalResponse(http.StatusCreated, nil)
+			resp.SetTerminalResponse(http.StatusOK, result, nil)
+
+			return
+		},
+		Get: func(
+			ctx context.Context,
+			planeName string,
+			resourceProviderName string,
+			options *v20231001preview.ResourceProvidersClientGetOptions, // Add this parameter
+		) (resp azfake.Responder[v20231001preview.ResourceProvidersClientGetResponse], errResp azfake.ErrorResponder) {
+			response := v20231001preview.ResourceProvidersClientGetResponse{
+				ResourceProviderResource: v20231001preview.ResourceProviderResource{
+					Name: to.Ptr(resourceProviderName),
+				},
+			}
+			resp.SetResponse(http.StatusInternalServerError, response, nil)
+			return
+		},
+	}
+	return resourceProvidersServerInternalError
+}