From 51066cacc28b840e98ce5214c6387959f38ccbe6 Mon Sep 17 00:00:00 2001 From: nithyatsu Date: Tue, 3 Dec 2024 12:42:15 -0800 Subject: [PATCH 01/10] add create resource type command --- pkg/cli/clivalidation.go | 25 +++ pkg/cli/cmd/resourcetype/create/create.go | 189 ++++++++++++++++++++++ 2 files changed, 214 insertions(+) create mode 100644 pkg/cli/cmd/resourcetype/create/create.go diff --git a/pkg/cli/clivalidation.go b/pkg/cli/clivalidation.go index 76addbe5ec..2c5fe77b0d 100644 --- a/pkg/cli/clivalidation.go +++ b/pkg/cli/clivalidation.go @@ -454,6 +454,31 @@ func ReadResourceGroupNameArgs(cmd *cobra.Command, args []string) (string, error return name, err } +// RequireResourceTypeNameArgs is used by commands that require specifying a type name using positional args. +// + +// RequireResourceTypeNameArgs reads the resource type name from the command line arguments and returns an error if the name +// is not provided or is empty. It also handles any errors that may occur while reading the resource type name. +func RequireResourceTypeNameArgs(cmd *cobra.Command, args []string) (string, error) { + resourceType, err := ReadResourceTypeNameArgs(cmd, args) + if err != nil { + return "", err + } + if resourceType == "" { + return "", fmt.Errorf("resource type name is not provided or is empty ") + } + + return resourceType, nil +} + +// ReadResourceTypeNameArgs is used to get the resource type name that is supplied as the first argument +func ReadResourceTypeNameArgs(cmd *cobra.Command, args []string) (string, error) { + if len(args) < 1 { + return "", errors.New("no resource type name provided") + } + return args[0], nil +} + // RequireWorkspaceArgs is used by commands that require an existing workspace either set as the default, // or specified as a positional arg, or specified using the 'workspace' flag. // diff --git a/pkg/cli/cmd/resourcetype/create/create.go b/pkg/cli/cmd/resourcetype/create/create.go new file mode 100644 index 0000000000..40ec34eed9 --- /dev/null +++ b/pkg/cli/cmd/resourcetype/create/create.go @@ -0,0 +1,189 @@ +/* +Copyright 2023 The Radius Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package create + +import ( + "context" + + v1 "github.com/radius-project/radius/pkg/armrpc/api/v1" + "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/connections" + "github.com/radius-project/radius/pkg/cli/framework" + "github.com/radius-project/radius/pkg/cli/manifest" + "github.com/radius-project/radius/pkg/cli/output" + "github.com/radius-project/radius/pkg/cli/workspaces" + "github.com/radius-project/radius/pkg/ucp/api/v20231001preview" + "github.com/spf13/cobra" +) + +// 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) + + cmd := &cobra.Command{ + Use: "create [input]", + Short: "Create or update a resource type", + Long: `Create or update a resource type from a resource provider manifest. + +Resource types are user defined types such as 'Mycompany.Messaging/plaid'. + +Creating a resource type defines a new type that can be used in applications. + +Input can be passed in using a JSON or YAML file using the --from-file option. +`, + Example: ` +# Create a resource type from YAML file +rad resource-type create myType --from-file /path/to/input.yaml + +# Create a resource type from JSON file +rad resource-type create myType --from-file /path/to/input.json +`, + Args: cobra.ExactArgs(0), + RunE: framework.RunCommand(runner), + } + + commonflags.AddOutputFlag(cmd) + commonflags.AddWorkspaceFlag(cmd) + commonflags.AddFromFileFlagVar(cmd, &runner.ResourceProviderManifestFilePath) + _ = cmd.MarkFlagRequired("from-file") + _ = cmd.MarkFlagFilename("from-file", "yaml", "json") + + return cmd, runner +} + +// Runner is the Runner implementation for the `rad resourceprovider create` command. +type Runner struct { + ConnectionFactory connections.Factory + ConfigHolder *framework.ConfigHolder + Output output.Interface + Format string + Workspace *workspaces.Workspace + + ResourceProviderManifestFilePath string + ResourceProvider *manifest.ResourceProvider + ResourceTypeName string +} + +// NewRunner creates an instance of the runner for the `rad resourceprovider create` command. +func NewRunner(factory framework.Factory) *Runner { + return &Runner{ + ConnectionFactory: factory.GetConnectionFactory(), + ConfigHolder: factory.GetConfigHolder(), + Output: factory.GetOutput(), + } +} + +// Validate runs validation for the `rad resourceprovider create` command. +func (r *Runner) Validate(cmd *cobra.Command, args []string) error { + // Validate command line args and + resourceTypeName, err := cli.RequireResourceTypeNameArgs(cmd, args) + if err != nil { + return err + } + r.ResourceTypeName = resourceTypeName + + workspace, err := cli.RequireWorkspace(cmd, r.ConfigHolder.Config, r.ConfigHolder.DirectoryConfig) + if err != nil { + return err + } + r.Workspace = workspace + + format, err := cli.RequireOutput(cmd) + if err != nil { + return err + } + r.Format = format + + r.ResourceProvider, err = manifest.ReadFile(r.ResourceProviderManifestFilePath) + if err != nil { + return err + } + + resourcesTypes := r.ResourceProvider.Types + if _, ok := resourcesTypes[r.ResourceTypeName]; !ok { + return clierrors.Message("Resource type %q not found in the manifest", r.ResourceTypeName) + } + + return nil +} + +// Run runs the `rad resourcetype create` command. +func (r *Runner) Run(ctx context.Context) error { + client, err := r.ConnectionFactory.CreateApplicationsManagementClient(ctx, *r.Workspace) + if err != nil { + return err + } + + r.Output.LogInfo("Checking resource provider %q exists", r.ResourceProvider.Name) + _, err = client.GetResourceProvider(ctx, "local", r.ResourceProvider.Name) + + if clients.Is404Error(err) { + return clierrors.Message("Resource Provider %q not found. Please create using `rad resorce-provider create` before creating a resource-type", r.ResourceProvider.Name) + } else if err != nil { + return err + } + + // The location resource contains references to all of the resource types and API versions that the resource provider supports. + // We're instantiating the struct here so we can update it as we loop. + locationResource := v20231001preview.LocationResource{ + Properties: &v20231001preview.LocationProperties{ + ResourceTypes: map[string]*v20231001preview.LocationResourceType{}, + }, + } + + resourceType := r.ResourceProvider.Types[r.ResourceTypeName] + r.Output.LogInfo("Creating resource type %s/%s", r.ResourceProvider.Name, r.ResourceTypeName) + _, err = client.CreateOrUpdateResourceType(ctx, "local", r.ResourceProvider.Name, r.ResourceTypeName, &v20231001preview.ResourceTypeResource{ + Properties: &v20231001preview.ResourceTypeProperties{ + DefaultAPIVersion: resourceType.DefaultAPIVersion, + }, + }) + if err != nil { + return err + } + + locationResourceType := &v20231001preview.LocationResourceType{ + APIVersions: map[string]map[string]any{}, + } + + for apiVersionName := range resourceType.APIVersions { + r.Output.LogInfo("Creating API Version %s/%s@%s", r.ResourceProvider.Name, r.ResourceTypeName, apiVersionName) + _, err := client.CreateOrUpdateAPIVersion(ctx, "local", r.ResourceProvider.Name, r.ResourceTypeName, apiVersionName, &v20231001preview.APIVersionResource{ + Properties: &v20231001preview.APIVersionProperties{}, + }) + if err != nil { + return err + } + + locationResourceType.APIVersions[apiVersionName] = map[string]any{} + } + + locationResource.Properties.ResourceTypes[r.ResourceTypeName] = locationResourceType + + r.Output.LogInfo("Creating location %s/%s", r.ResourceProvider.Name, v1.LocationGlobal) + _, err = client.CreateOrUpdateLocation(ctx, "local", r.ResourceProvider.Name, v1.LocationGlobal, &locationResource) + if err != nil { + return err + } + + r.Output.LogInfo("Resource type %s/%s created successfully", r.ResourceProvider.Name, r.ResourceTypeName) + + return nil +} From 68c276740312ec6e3844ef85a41d966b0183e32c Mon Sep 17 00:00:00 2001 From: nithyatsu Date: Fri, 3 Jan 2025 15:24:58 -0800 Subject: [PATCH 02/10] add a registerType function --- pkg/cli/manifest/registermanifest.go | 62 ++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/pkg/cli/manifest/registermanifest.go b/pkg/cli/manifest/registermanifest.go index 9ac98c6d7f..c605cfdc38 100644 --- a/pkg/cli/manifest/registermanifest.go +++ b/pkg/cli/manifest/registermanifest.go @@ -161,6 +161,68 @@ func RegisterDirectory(ctx context.Context, clientFactory *v20231001preview.Clie return nil } +// 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) + } + + logIfEnabled(logger, "Creating resource type %s/%s", resourceProvider.Name, typeName) + resourceTypePoller, err := clientFactory.NewResourceTypesClient().BeginCreateOrUpdate(ctx, planeName, resourceProvider.Name, typeName, v20231001preview.ResourceTypeResource{ + Properties: &v20231001preview.ResourceTypeProperties{ + DefaultAPIVersion: resourceProvider.Types[typeName].DefaultAPIVersion, + }, + }, nil) + if err != nil { + return err + } + + _, err = resourceTypePoller.PollUntilDone(ctx, nil) + if err != nil { + return err + } + + // get the existing location resource and update it + locationResourceGetResponse, err := clientFactory.NewLocationsClient().Get(ctx, planeName, resourceProvider.Name, v1.LocationGlobal, nil) + if err != nil { + return err + } + + locationResource := locationResourceGetResponse.LocationResource + locationResource.Properties.ResourceTypes[typeName] = &v20231001preview.LocationResourceType{ + APIVersions: map[string]map[string]any{ + *resourceType.DefaultAPIVersion: {}, + }, + } + + //set it back to resource provider + logIfEnabled(logger, "Updating location %s/%s", resourceProvider.Name, v1.LocationGlobal) + locationPoller, err := clientFactory.NewLocationsClient().BeginCreateOrUpdate(ctx, planeName, resourceProvider.Name, v1.LocationGlobal, locationResource, nil) + if err != nil { + return err + } + + _, err = locationPoller.PollUntilDone(ctx, nil) + if err != nil { + return err + } + + return nil +} + // Define an optional logger to prevent nil pointer dereference func logIfEnabled(logger func(format string, args ...any), format string, args ...any) { if logger != nil { From 657ed957f6201d61dc56547bcc0435c69d149fd3 Mon Sep 17 00:00:00 2001 From: nithyatsu Date: Mon, 6 Jan 2025 13:04:45 -0800 Subject: [PATCH 03/10] add create resource-type --- cmd/rad/cmd/root.go | 4 + pkg/cli/cmd/resourcetype/create/create.go | 103 ++++++++---------- .../cmd/resourcetype/create/create_test.go | 97 +++++++++++++++++ .../testdata/missing-required-field.yaml | 6 + .../resourcetype/create/testdata/valid.yaml | 12 ++ pkg/cli/manifest/registermanifest.go | 11 +- pkg/cli/manifest/testclientfactory.go | 23 +++- 7 files changed, 197 insertions(+), 59 deletions(-) create mode 100644 pkg/cli/cmd/resourcetype/create/create_test.go create mode 100644 pkg/cli/cmd/resourcetype/create/testdata/missing-required-field.yaml create mode 100644 pkg/cli/cmd/resourcetype/create/testdata/valid.yaml diff --git a/cmd/rad/cmd/root.go b/cmd/rad/cmd/root.go index 87c9f9b563..e360d5ba00 100644 --- a/cmd/rad/cmd/root.go +++ b/cmd/rad/cmd/root.go @@ -62,6 +62,7 @@ import ( resourceprovider_delete "github.com/radius-project/radius/pkg/cli/cmd/resourceprovider/delete" resourceprovider_list "github.com/radius-project/radius/pkg/cli/cmd/resourceprovider/list" resourceprovider_show "github.com/radius-project/radius/pkg/cli/cmd/resourceprovider/show" + resourcetype_create "github.com/radius-project/radius/pkg/cli/cmd/resourcetype/create" resourcetype_delete "github.com/radius-project/radius/pkg/cli/cmd/resourcetype/delete" resourcetype_list "github.com/radius-project/radius/pkg/cli/cmd/resourcetype/list" resourcetype_show "github.com/radius-project/radius/pkg/cli/cmd/resourcetype/show" @@ -273,6 +274,9 @@ func initSubCommands() { resourceTypeDeleteCmd, _ := resourcetype_delete.NewCommand(framework) resourceTypeCmd.AddCommand(resourceTypeDeleteCmd) + resourceTypeCreateCmd, _ := resourcetype_create.NewCommand(framework) + resourceTypeCmd.AddCommand(resourceTypeCreateCmd) + listRecipeCmd, _ := recipe_list.NewCommand(framework) recipeCmd.AddCommand(listRecipeCmd) diff --git a/pkg/cli/cmd/resourcetype/create/create.go b/pkg/cli/cmd/resourcetype/create/create.go index 40ec34eed9..792a96f4e6 100644 --- a/pkg/cli/cmd/resourcetype/create/create.go +++ b/pkg/cli/cmd/resourcetype/create/create.go @@ -19,16 +19,17 @@ package create import ( "context" - v1 "github.com/radius-project/radius/pkg/armrpc/api/v1" + aztoken "github.com/radius-project/radius/pkg/azure/tokencredentials" + "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/connections" + "github.com/radius-project/radius/pkg/cli/cmd/resourcetype/common" "github.com/radius-project/radius/pkg/cli/framework" "github.com/radius-project/radius/pkg/cli/manifest" "github.com/radius-project/radius/pkg/cli/output" "github.com/radius-project/radius/pkg/cli/workspaces" + "github.com/radius-project/radius/pkg/sdk" "github.com/radius-project/radius/pkg/ucp/api/v20231001preview" "github.com/spf13/cobra" ) @@ -55,38 +56,39 @@ rad resource-type create myType --from-file /path/to/input.yaml # Create a resource type from JSON file rad resource-type create myType --from-file /path/to/input.json `, - Args: cobra.ExactArgs(0), + Args: cobra.ExactArgs(1), RunE: framework.RunCommand(runner), } commonflags.AddOutputFlag(cmd) commonflags.AddWorkspaceFlag(cmd) commonflags.AddFromFileFlagVar(cmd, &runner.ResourceProviderManifestFilePath) - _ = cmd.MarkFlagRequired("from-file") - _ = cmd.MarkFlagFilename("from-file", "yaml", "json") return cmd, runner } // Runner is the Runner implementation for the `rad resourceprovider create` command. type Runner struct { - ConnectionFactory connections.Factory - ConfigHolder *framework.ConfigHolder - Output output.Interface - Format string - Workspace *workspaces.Workspace + UCPClientFactory *v20231001preview.ClientFactory + ConfigHolder *framework.ConfigHolder + Output output.Interface + Format string + Workspace *workspaces.Workspace ResourceProviderManifestFilePath string ResourceProvider *manifest.ResourceProvider ResourceTypeName string + Logger func(format string, args ...any) } // NewRunner creates an instance of the runner for the `rad resourceprovider create` command. func NewRunner(factory framework.Factory) *Runner { return &Runner{ - ConnectionFactory: factory.GetConnectionFactory(), - ConfigHolder: factory.GetConfigHolder(), - Output: factory.GetOutput(), + ConfigHolder: factory.GetConfigHolder(), + Output: factory.GetOutput(), + Logger: func(format string, args ...any) { + output.LogInfo(format, args...) + }, } } @@ -126,64 +128,53 @@ func (r *Runner) Validate(cmd *cobra.Command, args []string) error { // Run runs the `rad resourcetype create` command. func (r *Runner) Run(ctx context.Context) error { - client, err := r.ConnectionFactory.CreateApplicationsManagementClient(ctx, *r.Workspace) - if err != nil { - return err + // Initialize the client factory if it hasn't been set externally. + // This allows for flexibility where a test UCPClientFactory can be set externally during testing. + if r.UCPClientFactory == nil { + err := r.initializeClientFactory(ctx, r.Workspace) + if err != nil { + return err + } } - r.Output.LogInfo("Checking resource provider %q exists", r.ResourceProvider.Name) - _, err = client.GetResourceProvider(ctx, "local", r.ResourceProvider.Name) - - if clients.Is404Error(err) { - return clierrors.Message("Resource Provider %q not found. Please create using `rad resorce-provider create` before creating a resource-type", r.ResourceProvider.Name) - } else if err != nil { - return err + //response, err := r.UCPClientFactory.NewResourceProvidersClient().Get(ctx, "local", r.ResourceProvider.Name, nil) + response, err := r.UCPClientFactory.NewResourceTypesClient().Get(ctx, "local", r.ResourceProvider.Name, r.ResourceTypeName, nil) + if err != nil { + r.Output.LogInfo("Resource provider %q not found.", r.ResourceProvider.Name) + if err := manifest.RegisterFile(ctx, r.UCPClientFactory, "local", r.ResourceProviderManifestFilePath, r.Logger); err != nil { + return err + } + } else { + r.Output.LogInfo("Resource provider %q found. Registering resource type %q.", r.ResourceProvider.Name, r.ResourceTypeName) + if err := manifest.RegisterType(ctx, r.UCPClientFactory, "local", r.ResourceProviderManifestFilePath, r.ResourceTypeName, r.Logger); err != nil { + return err + } } - // The location resource contains references to all of the resource types and API versions that the resource provider supports. - // We're instantiating the struct here so we can update it as we loop. - locationResource := v20231001preview.LocationResource{ - Properties: &v20231001preview.LocationProperties{ - ResourceTypes: map[string]*v20231001preview.LocationResourceType{}, - }, - } + // Add a blank line before printing the result. + r.Output.LogInfo("") - resourceType := r.ResourceProvider.Types[r.ResourceTypeName] - r.Output.LogInfo("Creating resource type %s/%s", r.ResourceProvider.Name, r.ResourceTypeName) - _, err = client.CreateOrUpdateResourceType(ctx, "local", r.ResourceProvider.Name, r.ResourceTypeName, &v20231001preview.ResourceTypeResource{ - Properties: &v20231001preview.ResourceTypeProperties{ - DefaultAPIVersion: resourceType.DefaultAPIVersion, - }, - }) + err = r.Output.WriteFormatted(r.Format, response, common.GetResourceTypeTableFormat()) if err != nil { return err } - locationResourceType := &v20231001preview.LocationResourceType{ - APIVersions: map[string]map[string]any{}, - } - - for apiVersionName := range resourceType.APIVersions { - r.Output.LogInfo("Creating API Version %s/%s@%s", r.ResourceProvider.Name, r.ResourceTypeName, apiVersionName) - _, err := client.CreateOrUpdateAPIVersion(ctx, "local", r.ResourceProvider.Name, r.ResourceTypeName, apiVersionName, &v20231001preview.APIVersionResource{ - Properties: &v20231001preview.APIVersionProperties{}, - }) - if err != nil { - return err - } + return nil +} - locationResourceType.APIVersions[apiVersionName] = map[string]any{} +func (r *Runner) initializeClientFactory(ctx context.Context, workspace *workspaces.Workspace) error { + connection, err := workspace.Connect(ctx) + if err != nil { + return err } - locationResource.Properties.ResourceTypes[r.ResourceTypeName] = locationResourceType + clientOptions := sdk.NewClientOptions(connection) - r.Output.LogInfo("Creating location %s/%s", r.ResourceProvider.Name, v1.LocationGlobal) - _, err = client.CreateOrUpdateLocation(ctx, "local", r.ResourceProvider.Name, v1.LocationGlobal, &locationResource) + clientFactory, err := v20231001preview.NewClientFactory(&aztoken.AnonymousCredential{}, clientOptions) if err != nil { return err } - r.Output.LogInfo("Resource type %s/%s created successfully", r.ResourceProvider.Name, r.ResourceTypeName) - + r.UCPClientFactory = clientFactory return nil } diff --git a/pkg/cli/cmd/resourcetype/create/create_test.go b/pkg/cli/cmd/resourcetype/create/create_test.go new file mode 100644 index 0000000000..79450e0c52 --- /dev/null +++ b/pkg/cli/cmd/resourcetype/create/create_test.go @@ -0,0 +1,97 @@ +/* +Copyright 2023 The Radius Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package create + +import ( + "bytes" + "context" + "fmt" + "testing" + + "github.com/radius-project/radius/pkg/cli/framework" + "github.com/radius-project/radius/pkg/cli/manifest" + "github.com/radius-project/radius/pkg/cli/output" + "github.com/radius-project/radius/pkg/cli/workspaces" + "github.com/radius-project/radius/test/radcli" + "github.com/stretchr/testify/require" +) + +func Test_CommandValidation(t *testing.T) { + radcli.SharedCommandValidation(t, NewCommand) +} + +func Test_Validate(t *testing.T) { + config := radcli.LoadConfigWithWorkspace(t) + + testcases := []radcli.ValidateInput{ + { + Name: "Valid", + Input: []string{"coolResources", "--from-file", "testdata/valid.yaml"}, + ExpectedValid: true, + ConfigHolder: framework.ConfigHolder{Config: config}, + }, + { + Name: "Invalid: Error in manifest", + Input: []string{"testResources", "--from-file", "testdata/missing-required-field.yaml"}, + ExpectedValid: false, + ConfigHolder: framework.ConfigHolder{Config: config}, + }, + { + Name: "Invalid: missing arguments", + Input: []string{"--from-file", "testdata/valid.yaml"}, + ExpectedValid: false, + ConfigHolder: framework.ConfigHolder{Config: config}, + }, + } + + radcli.SharedValidateValidation(t, NewCommand, testcases) +} + +func Test_Run(t *testing.T) { + t.Run("Success: resource type created", func(t *testing.T) { + + resourceProviderData, err := manifest.ReadFile("testdata/valid.yaml") + require.NoError(t, err) + + expectedResourceType := "testResources" + + clientFactory, err := manifest.NewTestClientFactory() + 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.NoError(t, err) + + logOutput := logBuffer.String() + require.Contains(t, logOutput, fmt.Sprintf("Resource type %s/%s created successfully", resourceProviderData.Name, expectedResourceType)) + }) +} diff --git a/pkg/cli/cmd/resourcetype/create/testdata/missing-required-field.yaml b/pkg/cli/cmd/resourcetype/create/testdata/missing-required-field.yaml new file mode 100644 index 0000000000..15ab61b11d --- /dev/null +++ b/pkg/cli/cmd/resourcetype/create/testdata/missing-required-field.yaml @@ -0,0 +1,6 @@ +types: + testResources: + apiVersions: + '2025-01-01-preview': + schema: {} + capabilities: ["Recipes"] \ No newline at end of file diff --git a/pkg/cli/cmd/resourcetype/create/testdata/valid.yaml b/pkg/cli/cmd/resourcetype/create/testdata/valid.yaml new file mode 100644 index 0000000000..49bc2b9116 --- /dev/null +++ b/pkg/cli/cmd/resourcetype/create/testdata/valid.yaml @@ -0,0 +1,12 @@ +name: CoolCompany.Resources +types: + testResources: + apiVersions: + '2025-01-01-preview': + schema: {} + capabilities: ["Recipes"] + coolResources: + apiVersions: + '2025-01-01-preview': + schema: {} + capabilities: ["Recipes"] \ No newline at end of file diff --git a/pkg/cli/manifest/registermanifest.go b/pkg/cli/manifest/registermanifest.go index c605cfdc38..df6ab830f5 100644 --- a/pkg/cli/manifest/registermanifest.go +++ b/pkg/cli/manifest/registermanifest.go @@ -201,15 +201,21 @@ func RegisterType(ctx context.Context, clientFactory *v20231001preview.ClientFac return err } + var defaultAPIVersion string + if resourceType.DefaultAPIVersion == nil { + defaultAPIVersion = "2023-10-01-preview" + } else { + defaultAPIVersion = *resourceType.DefaultAPIVersion + } locationResource := locationResourceGetResponse.LocationResource locationResource.Properties.ResourceTypes[typeName] = &v20231001preview.LocationResourceType{ APIVersions: map[string]map[string]any{ - *resourceType.DefaultAPIVersion: {}, + defaultAPIVersion: {}, }, } //set it back to resource provider - logIfEnabled(logger, "Updating location %s/%s", resourceProvider.Name, v1.LocationGlobal) + logIfEnabled(logger, "Updating location %s/%s with new resource type", resourceProvider.Name, v1.LocationGlobal) locationPoller, err := clientFactory.NewLocationsClient().BeginCreateOrUpdate(ctx, planeName, resourceProvider.Name, v1.LocationGlobal, locationResource, nil) if err != nil { return err @@ -220,6 +226,7 @@ func RegisterType(ctx context.Context, clientFactory *v20231001preview.ClientFac return err } + logIfEnabled(logger, "Resource type %s/%s created successfully", resourceProvider.Name, typeName) return nil } diff --git a/pkg/cli/manifest/testclientfactory.go b/pkg/cli/manifest/testclientfactory.go index 85e9117d1f..80c61ed721 100644 --- a/pkg/cli/manifest/testclientfactory.go +++ b/pkg/cli/manifest/testclientfactory.go @@ -138,8 +138,29 @@ func NewTestClientFactory() (*v20231001preview.ClientFactory, error) { return }, + Get: func( + ctx context.Context, + planeName string, + resourceProviderName string, + locationName string, + options *v20231001preview.LocationsClientGetOptions, + ) (resp azfake.Responder[v20231001preview.LocationsClientGetResponse], errResp azfake.ErrorResponder) { + response := v20231001preview.LocationsClientGetResponse{ + LocationResource: v20231001preview.LocationResource{ + Name: to.Ptr(locationName), + ID: to.Ptr("id"), + Properties: &v20231001preview.LocationProperties{ + ResourceTypes: map[string]*v20231001preview.LocationResourceType{}, + }, + }, + } + resp.SetResponse(http.StatusOK, response, nil) + return + }, } - + //func(ctx context.Context, planeName string, resourceProviderName string, locationName string, options *v20231001preview.LocationsClientGetOptions) + // (resp "github.com/Azure/azure-sdk-for-go/sdk/azcore/fake".Responder[v20231001preview.LocationsClientGetResponse], errResp "github.com/Azure/azure-sdk-for-go/sdk/azcore/fake".ErrorResponder) + //(ctx, planeName, resourceProvider.Name, v1.LocationGlobal, nil) serverFactory := ucpfake.ServerFactory{ ResourceProvidersServer: resourceProvidersServer, ResourceTypesServer: resourceTypesServer, From 019d77d8309bb542512f7c5c89a93cc40bd35e9e Mon Sep 17 00:00:00 2001 From: nithyatsu Date: Mon, 6 Jan 2025 15:57:19 -0800 Subject: [PATCH 04/10] add a test and refactor --- pkg/cli/clivalidation.go | 4 ++-- pkg/cli/cmd/resourcetype/create/create.go | 3 +-- pkg/cli/cmd/resourcetype/create/create_test.go | 6 +++--- .../create/testdata/missing-required-field.yaml | 6 ------ pkg/cli/cmd/resourcetype/create/testdata/valid.yaml | 8 ++++---- pkg/cli/manifest/registermanifest.go | 4 ++-- 6 files changed, 12 insertions(+), 19 deletions(-) delete mode 100644 pkg/cli/cmd/resourcetype/create/testdata/missing-required-field.yaml diff --git a/pkg/cli/clivalidation.go b/pkg/cli/clivalidation.go index 2c5fe77b0d..849725b7bf 100644 --- a/pkg/cli/clivalidation.go +++ b/pkg/cli/clivalidation.go @@ -465,7 +465,7 @@ func RequireResourceTypeNameArgs(cmd *cobra.Command, args []string) (string, err return "", err } if resourceType == "" { - return "", fmt.Errorf("resource type name is not provided or is empty ") + return "", fmt.Errorf("resource type name is not provided or is empty") } return resourceType, nil @@ -474,7 +474,7 @@ func RequireResourceTypeNameArgs(cmd *cobra.Command, args []string) (string, err // ReadResourceTypeNameArgs is used to get the resource type name that is supplied as the first argument func ReadResourceTypeNameArgs(cmd *cobra.Command, args []string) (string, error) { if len(args) < 1 { - return "", errors.New("no resource type name provided") + return "", errors.New("resource type name is provided") } return args[0], nil } diff --git a/pkg/cli/cmd/resourcetype/create/create.go b/pkg/cli/cmd/resourcetype/create/create.go index 792a96f4e6..39ecc16e3d 100644 --- a/pkg/cli/cmd/resourcetype/create/create.go +++ b/pkg/cli/cmd/resourcetype/create/create.go @@ -94,7 +94,6 @@ func NewRunner(factory framework.Factory) *Runner { // Validate runs validation for the `rad resourceprovider create` command. func (r *Runner) Validate(cmd *cobra.Command, args []string) error { - // Validate command line args and resourceTypeName, err := cli.RequireResourceTypeNameArgs(cmd, args) if err != nil { return err @@ -137,7 +136,7 @@ func (r *Runner) Run(ctx context.Context) error { } } - //response, err := r.UCPClientFactory.NewResourceProvidersClient().Get(ctx, "local", r.ResourceProvider.Name, nil) + // If resource provider does not exist or if there is any other error, first register the resource provider. response, err := r.UCPClientFactory.NewResourceTypesClient().Get(ctx, "local", r.ResourceProvider.Name, r.ResourceTypeName, nil) if err != nil { r.Output.LogInfo("Resource provider %q not found.", r.ResourceProvider.Name) diff --git a/pkg/cli/cmd/resourcetype/create/create_test.go b/pkg/cli/cmd/resourcetype/create/create_test.go index 79450e0c52..75eb5fe7ec 100644 --- a/pkg/cli/cmd/resourcetype/create/create_test.go +++ b/pkg/cli/cmd/resourcetype/create/create_test.go @@ -45,13 +45,13 @@ func Test_Validate(t *testing.T) { ConfigHolder: framework.ConfigHolder{Config: config}, }, { - Name: "Invalid: Error in manifest", - Input: []string{"testResources", "--from-file", "testdata/missing-required-field.yaml"}, + Name: "Invalid: resource type not present in manifest", + Input: []string{"myResources", "--from-file", "testdata/valid.yaml"}, ExpectedValid: false, ConfigHolder: framework.ConfigHolder{Config: config}, }, { - Name: "Invalid: missing arguments", + Name: "Invalid: missing resource type as argument", Input: []string{"--from-file", "testdata/valid.yaml"}, ExpectedValid: false, ConfigHolder: framework.ConfigHolder{Config: config}, diff --git a/pkg/cli/cmd/resourcetype/create/testdata/missing-required-field.yaml b/pkg/cli/cmd/resourcetype/create/testdata/missing-required-field.yaml deleted file mode 100644 index 15ab61b11d..0000000000 --- a/pkg/cli/cmd/resourcetype/create/testdata/missing-required-field.yaml +++ /dev/null @@ -1,6 +0,0 @@ -types: - testResources: - apiVersions: - '2025-01-01-preview': - schema: {} - capabilities: ["Recipes"] \ No newline at end of file diff --git a/pkg/cli/cmd/resourcetype/create/testdata/valid.yaml b/pkg/cli/cmd/resourcetype/create/testdata/valid.yaml index 49bc2b9116..220799cc23 100644 --- a/pkg/cli/cmd/resourcetype/create/testdata/valid.yaml +++ b/pkg/cli/cmd/resourcetype/create/testdata/valid.yaml @@ -2,11 +2,11 @@ name: CoolCompany.Resources types: testResources: apiVersions: - '2025-01-01-preview': + '2023-10-01-preview': schema: {} - capabilities: ["Recipes"] + capabilities: ["Recipes"] coolResources: apiVersions: - '2025-01-01-preview': + '2023-10-01-preview': schema: {} - capabilities: ["Recipes"] \ No newline at end of file + capabilities: ["Recipes"] \ No newline at end of file diff --git a/pkg/cli/manifest/registermanifest.go b/pkg/cli/manifest/registermanifest.go index df6ab830f5..da04cf13be 100644 --- a/pkg/cli/manifest/registermanifest.go +++ b/pkg/cli/manifest/registermanifest.go @@ -177,7 +177,7 @@ func RegisterType(ctx context.Context, clientFactory *v20231001preview.ClientFac // 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) + return fmt.Errorf("Type %s not found in manifest file %s", typeName, filePath) } logIfEnabled(logger, "Creating resource type %s/%s", resourceProvider.Name, typeName) @@ -195,7 +195,7 @@ func RegisterType(ctx context.Context, clientFactory *v20231001preview.ClientFac return err } - // get the existing location resource and update it + // get the existing location resource and update it with new resource type. We have to revisit this code once schema is finalized and validated. locationResourceGetResponse, err := clientFactory.NewLocationsClient().Get(ctx, planeName, resourceProvider.Name, v1.LocationGlobal, nil) if err != nil { return err From 5f2362aa387f10b847ea9ad73804d7fc5a053ffe Mon Sep 17 00:00:00 2001 From: nithyatsu Date: Tue, 7 Jan 2025 10:48:23 -0800 Subject: [PATCH 05/10] add test --- pkg/cli/clivalidation.go | 2 +- .../resourceprovider/create/create_test.go | 2 +- pkg/cli/cmd/resourcetype/create/create.go | 14 +-- .../cmd/resourcetype/create/create_test.go | 36 +++++++- pkg/cli/manifest/registermanifest.go | 1 - pkg/cli/manifest/registermanifest_test.go | 4 +- pkg/cli/manifest/testclientfactory.go | 89 ++++++++++++++----- 7 files changed, 112 insertions(+), 36 deletions(-) diff --git a/pkg/cli/clivalidation.go b/pkg/cli/clivalidation.go index 849725b7bf..da8dcc03a3 100644 --- a/pkg/cli/clivalidation.go +++ b/pkg/cli/clivalidation.go @@ -474,7 +474,7 @@ func RequireResourceTypeNameArgs(cmd *cobra.Command, args []string) (string, err // ReadResourceTypeNameArgs is used to get the resource type name that is supplied as the first argument func ReadResourceTypeNameArgs(cmd *cobra.Command, args []string) (string, error) { if len(args) < 1 { - return "", errors.New("resource type name is provided") + return "", errors.New("resource type name is not provided") } return args[0], nil } diff --git a/pkg/cli/cmd/resourceprovider/create/create_test.go b/pkg/cli/cmd/resourceprovider/create/create_test.go index d74123e709..4f4f6015de 100644 --- a/pkg/cli/cmd/resourceprovider/create/create_test.go +++ b/pkg/cli/cmd/resourceprovider/create/create_test.go @@ -76,7 +76,7 @@ func Test_Run(t *testing.T) { expectedResourceType := "testResources" expectedAPIVersion := "2025-01-01-preview" - clientFactory, err := manifest.NewTestClientFactory() + clientFactory, err := manifest.NewTestClientFactory(manifest.WithResourceProviderServerNoError) require.NoError(t, err) var logBuffer bytes.Buffer diff --git a/pkg/cli/cmd/resourcetype/create/create.go b/pkg/cli/cmd/resourcetype/create/create.go index 39ecc16e3d..2cdb351a77 100644 --- a/pkg/cli/cmd/resourcetype/create/create.go +++ b/pkg/cli/cmd/resourcetype/create/create.go @@ -19,8 +19,6 @@ package create import ( "context" - aztoken "github.com/radius-project/radius/pkg/azure/tokencredentials" - "github.com/radius-project/radius/pkg/cli" "github.com/radius-project/radius/pkg/cli/clierrors" "github.com/radius-project/radius/pkg/cli/cmd/commonflags" @@ -32,6 +30,8 @@ import ( "github.com/radius-project/radius/pkg/sdk" "github.com/radius-project/radius/pkg/ucp/api/v20231001preview" "github.com/spf13/cobra" + + aztoken "github.com/radius-project/radius/pkg/azure/tokencredentials" ) // NewCommand creates an instance of the `rad resource-type create` command and runner. @@ -137,16 +137,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.NewResourceTypesClient().Get(ctx, "local", r.ResourceProvider.Name, r.ResourceTypeName, nil) + 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 err := manifest.RegisterFile(ctx, r.UCPClientFactory, "local", r.ResourceProviderManifestFilePath, r.Logger); err != nil { - return err + if registerErr := manifest.RegisterFile(ctx, r.UCPClientFactory, "local", r.ResourceProviderManifestFilePath, r.Logger); err != nil { + return registerErr } } else { r.Output.LogInfo("Resource provider %q found. Registering resource type %q.", r.ResourceProvider.Name, r.ResourceTypeName) - if err := manifest.RegisterType(ctx, r.UCPClientFactory, "local", r.ResourceProviderManifestFilePath, r.ResourceTypeName, r.Logger); err != nil { - return err + if registerErr := manifest.RegisterType(ctx, r.UCPClientFactory, "local", r.ResourceProviderManifestFilePath, r.ResourceTypeName, r.Logger); err != nil { + return registerErr } } diff --git a/pkg/cli/cmd/resourcetype/create/create_test.go b/pkg/cli/cmd/resourcetype/create/create_test.go index 75eb5fe7ec..8b78651e68 100644 --- a/pkg/cli/cmd/resourcetype/create/create_test.go +++ b/pkg/cli/cmd/resourcetype/create/create_test.go @@ -63,13 +63,12 @@ func Test_Validate(t *testing.T) { func Test_Run(t *testing.T) { t.Run("Success: resource type created", func(t *testing.T) { - resourceProviderData, err := manifest.ReadFile("testdata/valid.yaml") require.NoError(t, err) expectedResourceType := "testResources" - clientFactory, err := manifest.NewTestClientFactory() + clientFactory, err := manifest.NewTestClientFactory(manifest.WithResourceProviderServerNoError) require.NoError(t, err) var logBuffer bytes.Buffer @@ -92,6 +91,39 @@ func Test_Run(t *testing.T) { require.NoError(t, err) logOutput := logBuffer.String() + 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) + + expectedResourceType := "testResources" + + clientFactory, err := manifest.NewTestClientFactory(manifest.WithResourceProviderServerNotFoundError) + 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, + } + + _ = runner.Run(context.Background()) + logOutput := logBuffer.String() + require.Contains(t, logOutput, fmt.Sprintf("Creating resource provider %s", runner.ResourceProvider.Name)) + + }) + } diff --git a/pkg/cli/manifest/registermanifest.go b/pkg/cli/manifest/registermanifest.go index da04cf13be..75a5e94598 100644 --- a/pkg/cli/manifest/registermanifest.go +++ b/pkg/cli/manifest/registermanifest.go @@ -214,7 +214,6 @@ func RegisterType(ctx context.Context, clientFactory *v20231001preview.ClientFac }, } - //set it back to resource provider logIfEnabled(logger, "Updating location %s/%s with new resource type", resourceProvider.Name, v1.LocationGlobal) locationPoller, err := clientFactory.NewLocationsClient().BeginCreateOrUpdate(ctx, planeName, resourceProvider.Name, v1.LocationGlobal, locationResource, nil) if err != nil { diff --git a/pkg/cli/manifest/registermanifest_test.go b/pkg/cli/manifest/registermanifest_test.go index cc2c1ec525..810fddce9a 100644 --- a/pkg/cli/manifest/registermanifest_test.go +++ b/pkg/cli/manifest/registermanifest_test.go @@ -73,7 +73,7 @@ func TestRegisterDirectory(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - clientFactory, err := NewTestClientFactory() + clientFactory, err := NewTestClientFactory(WithResourceProviderServerNoError) require.NoError(t, err) err = RegisterDirectory(context.Background(), clientFactory, tt.planeName, tt.directoryPath, nil) @@ -129,7 +129,7 @@ func TestRegisterFile(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - clientFactory, err := NewTestClientFactory() + clientFactory, err := NewTestClientFactory(WithResourceProviderServerNoError) require.NoError(t, err, "Failed to create client factory") var logBuffer bytes.Buffer diff --git a/pkg/cli/manifest/testclientfactory.go b/pkg/cli/manifest/testclientfactory.go index 80c61ed721..a48506feda 100644 --- a/pkg/cli/manifest/testclientfactory.go +++ b/pkg/cli/manifest/testclientfactory.go @@ -30,8 +30,31 @@ import ( ) // NewTestClientFactory creates a new client factory for testing purposes. -func NewTestClientFactory() (*v20231001preview.ClientFactory, error) { - // Create fake servers for each client +func NewTestClientFactory(resourceProvidersServer func() ucpfake.ResourceProvidersServer) (*v20231001preview.ClientFactory, error) { + serverFactory := ucpfake.ServerFactory{ + ResourceProvidersServer: resourceProvidersServer(), + ResourceTypesServer: WithResourceTypeServerNoError(), + APIVersionsServer: WithAPIVersionServerNoError(), + LocationsServer: WithLocationServerNoError(), + } + + serverFactoryTransport := ucpfake.NewServerFactoryTransport(&serverFactory) + + clientOptions := &armpolicy.ClientOptions{ + ClientOptions: policy.ClientOptions{ + Transport: serverFactoryTransport, + }, + } + + clientFactory, err := v20231001preview.NewClientFactory(&azfake.TokenCredential{}, clientOptions) + if err != nil { + return nil, err + } + + return clientFactory, err +} + +func WithResourceProviderServerNoError() ucpfake.ResourceProvidersServer { resourceProvidersServer := ucpfake.ResourceProvidersServer{ BeginCreateOrUpdate: func( ctx context.Context, @@ -64,7 +87,10 @@ func NewTestClientFactory() (*v20231001preview.ClientFactory, error) { return }, } + return resourceProvidersServer +} +func WithResourceTypeServerNoError() ucpfake.ResourceTypesServer { resourceTypesServer := ucpfake.ResourceTypesServer{ BeginCreateOrUpdate: func( ctx context.Context, @@ -99,7 +125,10 @@ func NewTestClientFactory() (*v20231001preview.ClientFactory, error) { return }, } + return resourceTypesServer +} +func WithAPIVersionServerNoError() ucpfake.APIVersionsServer { apiVersionsServer := ucpfake.APIVersionsServer{ BeginCreateOrUpdate: func( ctx context.Context, @@ -119,7 +148,10 @@ func NewTestClientFactory() (*v20231001preview.ClientFactory, error) { return }, } + return apiVersionsServer +} +func WithLocationServerNoError() ucpfake.LocationsServer { locationsServer := ucpfake.LocationsServer{ BeginCreateOrUpdate: func( ctx context.Context, @@ -158,28 +190,41 @@ func NewTestClientFactory() (*v20231001preview.ClientFactory, error) { return }, } - //func(ctx context.Context, planeName string, resourceProviderName string, locationName string, options *v20231001preview.LocationsClientGetOptions) - // (resp "github.com/Azure/azure-sdk-for-go/sdk/azcore/fake".Responder[v20231001preview.LocationsClientGetResponse], errResp "github.com/Azure/azure-sdk-for-go/sdk/azcore/fake".ErrorResponder) - //(ctx, planeName, resourceProvider.Name, v1.LocationGlobal, nil) - serverFactory := ucpfake.ServerFactory{ - ResourceProvidersServer: resourceProvidersServer, - ResourceTypesServer: resourceTypesServer, - APIVersionsServer: apiVersionsServer, - LocationsServer: locationsServer, - } + return locationsServer +} - serverFactoryTransport := ucpfake.NewServerFactoryTransport(&serverFactory) +func WithResourceProviderServerNotFoundError() ucpfake.ResourceProvidersServer { + resourceProvidersNotFoundServer := 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) - clientOptions := &armpolicy.ClientOptions{ - ClientOptions: policy.ClientOptions{ - Transport: serverFactoryTransport, + 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.StatusNotFound, response, nil) + return }, } - - clientFactory, err := v20231001preview.NewClientFactory(&azfake.TokenCredential{}, clientOptions) - if err != nil { - return nil, err - } - - return clientFactory, err + return resourceProvidersNotFoundServer } From e42745a5114c6925cb1a29f4ece6bdacd1693b88 Mon Sep 17 00:00:00 2001 From: nithyatsu Date: Tue, 7 Jan 2025 11:02:43 -0800 Subject: [PATCH 06/10] update comments --- .vscode/launch.json | 97 +++-------------------- pkg/cli/cmd/resourcetype/create/create.go | 22 ++--- 2 files changed, 20 insertions(+), 99 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 85a8aaa6fe..9567b9b3b4 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -1,7 +1,4 @@ { - // Use IntelliSense to learn about possible attributes. - // Hover to view descriptions of existing attributes. - // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387 "version": "0.2.0", "configurations": [ { @@ -9,10 +6,14 @@ "type": "go", "request": "launch", "mode": "auto", - "preLaunchTask": "Build Radius (all)", + "preLaunchTask": "Build Radius (all)", "program": "${workspaceFolder}/cmd/rad/main.go", "cwd": "${workspaceFolder}", - "args": [] + "args": [ + "resource-type", "create", "coolResources", "-f", "/Users/nithya/radius/radius/pkg/cli/cmd/resourceprovider/create/testdata/valid.yaml", + //"resource-provider", "create", "-f", "/Users/nithya/radius/radius/pkg/cli/cmd/resourceprovider/create/testdata/valid.yaml" + //"workspace","create","kubernetes","-w","ws3","-g","rg2","--force","-e","env1" + ] }, { "name": "Debug rad CLI (prompt for args)", @@ -23,94 +24,14 @@ "program": "${workspaceFolder}/cmd/rad", "args": "${input:cliArgs}", "console": "integratedTerminal" - }, - { - "name": "Launch Applications RP", - "type": "go", - "request": "launch", - "mode": "auto", - "preLaunchTask": "Build Radius (all)", - "program": "${workspaceFolder}/cmd/applications-rp/main.go", - }, - { - "name": "Launch Dynamic RP", - "type": "go", - "request": "launch", - "mode": "auto", - "preLaunchTask": "Build Radius (all)", - "program": "${workspaceFolder}/cmd/dynamic-rp/main.go", - }, - { - "name": "Launch UCP", - "type": "go", - "request": "launch", - "mode": "auto", - "preLaunchTask": "Build Radius (all)", - "program": "${workspaceFolder}/cmd/ucpd/main.go", - }, - { - "name": "Launch Controller", - "type": "go", - "request": "launch", - "mode": "auto", - "preLaunchTask": "Build Radius (all)", - "program": "${workspaceFolder}/cmd/controller/main.go", - "args": [ - "--cert-dir", - "" - ], - }, - { - "name": "Launch Deployment Engine", - "type": "coreclr", - "request": "launch", - "preLaunchTask": "Build Deployment Engine", - "program": "${workspaceFolder}/../deployment-engine/src/DeploymentEngine/bin/Debug/net6.0/arm-de.dll", - "args": [], - "cwd": "${workspaceFolder}/../deployment-engine/src/DeploymentEngine", - "stopAtEntry": false, - "env": { - "ASPNETCORE_URLS": "http://localhost:5017", - "ASPNETCORE_ENVIRONMENT": "Development", - "KUBERNETESBICEPEXTENSIBILITYURL": "http://localhost:5017/api", - "RADIUSBACKENDURI": "http://localhost:9000" - } - }, - { - "name": "Debug Bicep generator integration tests", - "type": "node", - "request": "launch", - "runtimeArgs": [ - "--inspect-brk", - "${workspaceRoot}/hack/bicep-types-radius/src/autorest.bicep/node_modules/.bin/jest", - "--runInBand", - "--no-cache" - ], - "cwd": "${workspaceFolder}/hack/bicep-types-radius/src/autorest.bicep/src", - "console": "integratedTerminal", - "internalConsoleOptions": "neverOpen", - "sourceMaps": true - } - ], - "compounds": [ - { - "name": "Launch Control Plane (all)", - "configurations": [ - "Launch UCP", - "Launch Applications RP", - "Launch Dynamic RP", - "Launch Controller", - "Launch Deployment Engine" - ], - "stopAll": true } ], "inputs": [ { "id": "cliArgs", "type": "promptString", - "description": "Args for launching Radius cli. Use --cwd to set the working directory.", - "default": "init --full" + "description": "Enter arguments for rad CLI", + "default": "" } ] -} +} \ No newline at end of file diff --git a/pkg/cli/cmd/resourcetype/create/create.go b/pkg/cli/cmd/resourcetype/create/create.go index 2cdb351a77..a8b6e8bf81 100644 --- a/pkg/cli/cmd/resourcetype/create/create.go +++ b/pkg/cli/cmd/resourcetype/create/create.go @@ -42,13 +42,13 @@ func NewCommand(factory framework.Factory) (*cobra.Command, framework.Runner) { Use: "create [input]", Short: "Create or update a resource type", Long: `Create or update a resource type from a resource provider manifest. - -Resource types are user defined types such as 'Mycompany.Messaging/plaid'. - -Creating a resource type defines a new type that can be used in applications. - -Input can be passed in using a JSON or YAML file using the --from-file option. -`, + + Resource types are user defined types such as 'Mycompany.Messaging/plaid'. + + Creating a resource type defines a new type that can be used in applications. + + Input can be passed in using a JSON or YAML file using the --from-file option. + `, Example: ` # Create a resource type from YAML file rad resource-type create myType --from-file /path/to/input.yaml @@ -67,7 +67,7 @@ rad resource-type create myType --from-file /path/to/input.json return cmd, runner } -// Runner is the Runner implementation for the `rad resourceprovider create` command. +// Runner is the Runner implementation for the `rad resource-type create` command. type Runner struct { UCPClientFactory *v20231001preview.ClientFactory ConfigHolder *framework.ConfigHolder @@ -81,7 +81,7 @@ type Runner struct { Logger func(format string, args ...any) } -// NewRunner creates an instance of the runner for the `rad resourceprovider create` command. +// NewRunner creates an instance of the runner for the `rad resource-type create` command. func NewRunner(factory framework.Factory) *Runner { return &Runner{ ConfigHolder: factory.GetConfigHolder(), @@ -92,7 +92,7 @@ func NewRunner(factory framework.Factory) *Runner { } } -// Validate runs validation for the `rad resourceprovider create` command. +// Validate runs validation for the `rad resource-type create` command. func (r *Runner) Validate(cmd *cobra.Command, args []string) error { resourceTypeName, err := cli.RequireResourceTypeNameArgs(cmd, args) if err != nil { @@ -125,7 +125,7 @@ func (r *Runner) Validate(cmd *cobra.Command, args []string) error { return nil } -// Run runs the `rad resourcetype create` command. +// Run runs the `rad resource-type create` command. func (r *Runner) Run(ctx context.Context) error { // Initialize the client factory if it hasn't been set externally. // This allows for flexibility where a test UCPClientFactory can be set externally during testing. From 8383d457a5d766e1cbd7f355709fe610d79dfa12 Mon Sep 17 00:00:00 2001 From: nithyatsu Date: Tue, 7 Jan 2025 11:04:48 -0800 Subject: [PATCH 07/10] revert local changes to launch.json --- .vscode/launch.json | 97 ++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 88 insertions(+), 9 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 9567b9b3b4..85a8aaa6fe 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -1,4 +1,7 @@ { + // Use IntelliSense to learn about possible attributes. + // Hover to view descriptions of existing attributes. + // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387 "version": "0.2.0", "configurations": [ { @@ -6,14 +9,10 @@ "type": "go", "request": "launch", "mode": "auto", - "preLaunchTask": "Build Radius (all)", + "preLaunchTask": "Build Radius (all)", "program": "${workspaceFolder}/cmd/rad/main.go", "cwd": "${workspaceFolder}", - "args": [ - "resource-type", "create", "coolResources", "-f", "/Users/nithya/radius/radius/pkg/cli/cmd/resourceprovider/create/testdata/valid.yaml", - //"resource-provider", "create", "-f", "/Users/nithya/radius/radius/pkg/cli/cmd/resourceprovider/create/testdata/valid.yaml" - //"workspace","create","kubernetes","-w","ws3","-g","rg2","--force","-e","env1" - ] + "args": [] }, { "name": "Debug rad CLI (prompt for args)", @@ -24,14 +23,94 @@ "program": "${workspaceFolder}/cmd/rad", "args": "${input:cliArgs}", "console": "integratedTerminal" + }, + { + "name": "Launch Applications RP", + "type": "go", + "request": "launch", + "mode": "auto", + "preLaunchTask": "Build Radius (all)", + "program": "${workspaceFolder}/cmd/applications-rp/main.go", + }, + { + "name": "Launch Dynamic RP", + "type": "go", + "request": "launch", + "mode": "auto", + "preLaunchTask": "Build Radius (all)", + "program": "${workspaceFolder}/cmd/dynamic-rp/main.go", + }, + { + "name": "Launch UCP", + "type": "go", + "request": "launch", + "mode": "auto", + "preLaunchTask": "Build Radius (all)", + "program": "${workspaceFolder}/cmd/ucpd/main.go", + }, + { + "name": "Launch Controller", + "type": "go", + "request": "launch", + "mode": "auto", + "preLaunchTask": "Build Radius (all)", + "program": "${workspaceFolder}/cmd/controller/main.go", + "args": [ + "--cert-dir", + "" + ], + }, + { + "name": "Launch Deployment Engine", + "type": "coreclr", + "request": "launch", + "preLaunchTask": "Build Deployment Engine", + "program": "${workspaceFolder}/../deployment-engine/src/DeploymentEngine/bin/Debug/net6.0/arm-de.dll", + "args": [], + "cwd": "${workspaceFolder}/../deployment-engine/src/DeploymentEngine", + "stopAtEntry": false, + "env": { + "ASPNETCORE_URLS": "http://localhost:5017", + "ASPNETCORE_ENVIRONMENT": "Development", + "KUBERNETESBICEPEXTENSIBILITYURL": "http://localhost:5017/api", + "RADIUSBACKENDURI": "http://localhost:9000" + } + }, + { + "name": "Debug Bicep generator integration tests", + "type": "node", + "request": "launch", + "runtimeArgs": [ + "--inspect-brk", + "${workspaceRoot}/hack/bicep-types-radius/src/autorest.bicep/node_modules/.bin/jest", + "--runInBand", + "--no-cache" + ], + "cwd": "${workspaceFolder}/hack/bicep-types-radius/src/autorest.bicep/src", + "console": "integratedTerminal", + "internalConsoleOptions": "neverOpen", + "sourceMaps": true + } + ], + "compounds": [ + { + "name": "Launch Control Plane (all)", + "configurations": [ + "Launch UCP", + "Launch Applications RP", + "Launch Dynamic RP", + "Launch Controller", + "Launch Deployment Engine" + ], + "stopAll": true } ], "inputs": [ { "id": "cliArgs", "type": "promptString", - "description": "Enter arguments for rad CLI", - "default": "" + "description": "Args for launching Radius cli. Use --cwd to set the working directory.", + "default": "init --full" } ] -} \ No newline at end of file +} From e4d9b42245237c10e3760ebda07a139a3e897dac Mon Sep 17 00:00:00 2001 From: nithyatsu Date: Wed, 8 Jan 2025 13:11:04 -0800 Subject: [PATCH 08/10] 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 +++++++--- .../cmd/resourcetype/create/create_test.go | 29 ++++++++++++++- pkg/cli/manifest/registermanifest.go | 12 ++----- pkg/cli/manifest/testclientfactory.go | 36 +++++++++++++++++++ 4 files changed, 80 insertions(+), 15 deletions(-) diff --git a/pkg/cli/cmd/resourcetype/create/create.go b/pkg/cli/cmd/resourcetype/create/create.go index a8b6e8bf81..4826010501 100644 --- a/pkg/cli/cmd/resourcetype/create/create.go +++ b/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) diff --git a/pkg/cli/cmd/resourcetype/create/create_test.go b/pkg/cli/cmd/resourcetype/create/create_test.go index 8b78651e68..d8c592f97c 100644 --- a/pkg/cli/cmd/resourcetype/create/create_test.go +++ b/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") + }) } diff --git a/pkg/cli/manifest/registermanifest.go b/pkg/cli/manifest/registermanifest.go index 75a5e94598..72ad93090c 100644 --- a/pkg/cli/manifest/registermanifest.go +++ b/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 } diff --git a/pkg/cli/manifest/testclientfactory.go b/pkg/cli/manifest/testclientfactory.go index a48506feda..3e91b789ed 100644 --- a/pkg/cli/manifest/testclientfactory.go +++ b/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 +} From 1469009146c071b5bcb1be6e2a1dbedad0d28c0c Mon Sep 17 00:00:00 2001 From: nithyatsu Date: Wed, 8 Jan 2025 13:14:55 -0800 Subject: [PATCH 09/10] nit --- pkg/cli/cmd/resourcetype/create/create_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cli/cmd/resourcetype/create/create_test.go b/pkg/cli/cmd/resourcetype/create/create_test.go index d8c592f97c..8d22ada7d8 100644 --- a/pkg/cli/cmd/resourcetype/create/create_test.go +++ b/pkg/cli/cmd/resourcetype/create/create_test.go @@ -151,6 +151,6 @@ func Test_Run(t *testing.T) { err = runner.Run(context.Background()) require.Error(t, err) - require.Contains(t, err.Error(), "Internal Error") + require.Contains(t, err.Error(), "unexpected status code 500.") }) } From c6eb041a3e9d6e6a73b34e6a7598bc61d95e386f Mon Sep 17 00:00:00 2001 From: nithyatsu Date: Wed, 8 Jan 2025 15:00:09 -0800 Subject: [PATCH 10/10] remove some comments and add a const. --- pkg/cli/cmd/resourceprovider/create/create.go | 1 - pkg/cli/cmd/resourcetype/create/create.go | 1 - pkg/cli/manifest/registermanifest.go | 2 +- 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/pkg/cli/cmd/resourceprovider/create/create.go b/pkg/cli/cmd/resourceprovider/create/create.go index 162bef3c76..97333b0c08 100644 --- a/pkg/cli/cmd/resourceprovider/create/create.go +++ b/pkg/cli/cmd/resourceprovider/create/create.go @@ -136,7 +136,6 @@ func (r *Runner) Run(ctx context.Context) error { return err } - // Add a blank line before printing the result. r.Output.LogInfo("") err = r.Output.WriteFormatted(r.Format, response, common.GetResourceProviderTableFormat()) diff --git a/pkg/cli/cmd/resourcetype/create/create.go b/pkg/cli/cmd/resourcetype/create/create.go index 4826010501..d8a24c3ac0 100644 --- a/pkg/cli/cmd/resourcetype/create/create.go +++ b/pkg/cli/cmd/resourcetype/create/create.go @@ -160,7 +160,6 @@ func (r *Runner) Run(ctx context.Context) error { } } - // Add a blank line before printing the result. r.Output.LogInfo("") err = r.Output.WriteFormatted(r.Format, response, common.GetResourceTypeTableFormat()) diff --git a/pkg/cli/manifest/registermanifest.go b/pkg/cli/manifest/registermanifest.go index 72ad93090c..2c2a54fb72 100644 --- a/pkg/cli/manifest/registermanifest.go +++ b/pkg/cli/manifest/registermanifest.go @@ -195,7 +195,7 @@ func RegisterType(ctx context.Context, clientFactory *v20231001preview.ClientFac var defaultAPIVersion string if resourceType.DefaultAPIVersion == nil { - 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 + defaultAPIVersion = v20231001preview.Version //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 }