Skip to content

Commit

Permalink
Resource type create - fixes (#8248)
Browse files Browse the repository at this point in the history
# Description

Fixes below issues in resource-type create
- APIversion for the created resource-type was blank. 
- Display of the resource-type created at the end of the command was
incorrect
- Added UTs that would have caught these issues.
- sample manifest updated to have just one resource-type

## Type of change


- This pull request fixes a bug in Radius and has an approved issue
(issue link required).
- This pull request is a minor refactor, code cleanup, test improvement,
or other maintenance task and doesn't change the functionality of Radius
(issue link optional)

Fixes: #issue_number

## Contributor checklist
Please verify that the PR meets the following requirements, where
applicable:

- [ NA ] An overview of proposed schema changes is included in a linked
GitHub issue.
- [ NA ] A design document PR is created in the [design-notes
repository](https://github.com/radius-project/design-notes/), if new
APIs are being introduced.
- [ NA ] If applicable, design document has been reviewed and approved
by Radius maintainers/approvers.
- [ NA ] A PR for the [samples
repository](https://github.com/radius-project/samples) is created, if
existing samples are affected by the changes in this PR.
- [ NA ] A PR for the [documentation
repository](https://github.com/radius-project/docs) is created, if the
changes in this PR affect the documentation or any user facing updates
are made.
- [ NA ] A PR for the [recipes
repository](https://github.com/radius-project/recipes) is created, if
existing recipes are affected by the changes in this PR.
  • Loading branch information
nithyatsu authored Jan 17, 2025
1 parent d56c866 commit 846f546
Show file tree
Hide file tree
Showing 10 changed files with 339 additions and 60 deletions.
10 changes: 10 additions & 0 deletions pkg/cli/clients/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,17 @@ import (
"encoding/json"
"errors"
"net/http"
"strings"

"github.com/Azure/azure-sdk-for-go/sdk/azcore"
v1 "github.com/radius-project/radius/pkg/armrpc/api/v1"
"github.com/radius-project/radius/pkg/corerp/api/v20231001preview"
)

const (
fakeServerNotFoundResponse = "unexpected status code 404. acceptable values are http.StatusOK"
)

// Is404Error returns true if the error is a 404 payload from an autorest operation.
//

Expand All @@ -38,6 +43,11 @@ func Is404Error(err error) bool {
return false
}

// NotFound Response from Fake Server - used for testing
if strings.Contains(err.Error(), fakeServerNotFoundResponse) {
return true
}

// The error might already be an ResponseError
responseError := &azcore.ResponseError{}
if errors.As(err, &responseError) && responseError.ErrorCode == v1.CodeNotFound || responseError.StatusCode == http.StatusNotFound {
Expand Down
6 changes: 6 additions & 0 deletions pkg/cli/clients/errors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,4 +62,10 @@ func TestIs404Error(t *testing.T) {
if Is404Error(nil) {
t.Errorf("Expected Is404Error to return false for nil error, but it returned true")
}

// Test with a fake server not found response
err = errors.New(fakeServerNotFoundResponse)
if !Is404Error(err) {
t.Errorf("Expected Is404Error to return true for fake server not found response, but it returned false")
}
}
26 changes: 26 additions & 0 deletions pkg/cli/cmd/resourcetype/common/resourcetype.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ limitations under the License.
package common

import (
"context"
"slices"

"github.com/radius-project/radius/pkg/cli/clients"
"github.com/radius-project/radius/pkg/cli/clierrors"
"github.com/radius-project/radius/pkg/cli/output"
"github.com/radius-project/radius/pkg/ucp/api/v20231001preview"
)
Expand Down Expand Up @@ -68,3 +73,24 @@ func GetResourceTypeTableFormat() output.FormatterOptions {
},
}
}

// GetResourceTypeDetails fetches the details of a resource type from the resource provider.
func GetResourceTypeDetails(ctx context.Context, resourceProviderName string, resourceTypeName string, client clients.ApplicationsManagementClient) (ResourceType, error) {
resourceProvider, err := client.GetResourceProviderSummary(ctx, "local", resourceProviderName)
if clients.Is404Error(err) {
return ResourceType{}, clierrors.Message("The resource provider %q was not found or has been deleted.", resourceProviderName)
} else if err != nil {
return ResourceType{}, err
}

resourceTypes := ResourceTypesForProvider(&resourceProvider)
idx := slices.IndexFunc(resourceTypes, func(rt ResourceType) bool {
return rt.Name == resourceProviderName+"/"+resourceTypeName
})

if idx < 0 {
return ResourceType{}, clierrors.Message("Resource type %q not found in resource provider %q.", resourceTypeName, *resourceProvider.Name)
}

return resourceTypes[idx], nil
}
90 changes: 90 additions & 0 deletions pkg/cli/cmd/resourcetype/common/resourcetype_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
/*
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 common

import (
"context"
"errors"
"testing"

"github.com/radius-project/radius/pkg/cli/clients"
"github.com/radius-project/radius/pkg/to"
"github.com/radius-project/radius/pkg/ucp/api/v20231001preview"
"github.com/radius-project/radius/test/radcli"
"github.com/stretchr/testify/require"
"go.uber.org/mock/gomock"
)

func Test_GetResourceTypeDetails(t *testing.T) {
t.Run("Get Resource Details Success", func(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

resourceProvider := v20231001preview.ResourceProviderSummary{
Name: to.Ptr("Applications.Test"),
ResourceTypes: map[string]*v20231001preview.ResourceProviderSummaryResourceType{
"exampleResources": {
APIVersions: map[string]map[string]any{
"2023-10-01-preview": {},
},
},
},
}

appManagementClient := clients.NewMockApplicationsManagementClient(ctrl)
appManagementClient.EXPECT().
GetResourceProviderSummary(gomock.Any(), "local", "Applications.Test").
Return(resourceProvider, nil).
Times(1)

res, err := GetResourceTypeDetails(context.Background(), "Applications.Test", "exampleResources", appManagementClient)
require.NoError(t, err)
require.Equal(t, "Applications.Test/exampleResources", res.Name)

})

t.Run("Get Resource Details Failure - Resource Provider Not found", func(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

appManagementClient := clients.NewMockApplicationsManagementClient(ctrl)
appManagementClient.EXPECT().
GetResourceProviderSummary(gomock.Any(), "local", "Applications.Test").
Return(v20231001preview.ResourceProviderSummary{}, radcli.Create404Error()).
Times(1)

_, err := GetResourceTypeDetails(context.Background(), "Applications.Test", "exampleResources", appManagementClient)

require.Error(t, err)
require.Equal(t, "The resource provider \"Applications.Test\" was not found or has been deleted.", err.Error())
})

t.Run("Get Resource Details Failures Other Than Not Found", func(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

appManagementClient := clients.NewMockApplicationsManagementClient(ctrl)
appManagementClient.EXPECT().
GetResourceProviderSummary(gomock.Any(), "local", "Applications.Test").
Return(v20231001preview.ResourceProviderSummary{}, errors.New("some error occurred")).
Times(1)

_, err := GetResourceTypeDetails(context.Background(), "Applications.Test", "exampleResources", appManagementClient)

require.Error(t, err)
})
}
44 changes: 27 additions & 17 deletions pkg/cli/cmd/resourcetype/create/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@ 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"
"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"
Expand All @@ -36,18 +36,14 @@ 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)

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.
Long: `Create or update a resource type from a resource type manifest.
Resource types are user defined types such as 'Mycompany.Messaging/plaid'.
Expand Down Expand Up @@ -75,11 +71,12 @@ rad resource-type create myType --from-file /path/to/input.json

// Runner is the Runner implementation for the `rad resource-type create` command.
type Runner struct {
UCPClientFactory *v20231001preview.ClientFactory
ConfigHolder *framework.ConfigHolder
Output output.Interface
Format string
Workspace *workspaces.Workspace
UCPClientFactory *v20231001preview.ClientFactory
ConnectionFactory connections.Factory
ConfigHolder *framework.ConfigHolder
Output output.Interface
Format string
Workspace *workspaces.Workspace

ResourceProviderManifestFilePath string
ResourceProvider *manifest.ResourceProvider
Expand All @@ -90,8 +87,9 @@ type Runner struct {
// 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(),
Output: factory.GetOutput(),
ConnectionFactory: factory.GetConnectionFactory(),
ConfigHolder: factory.GetConfigHolder(),
Output: factory.GetOutput(),
Logger: func(format string, args ...any) {
output.LogInfo(format, args...)
},
Expand Down Expand Up @@ -142,10 +140,9 @@ func (r *Runner) Run(ctx context.Context) error {
}
}

response, err := r.UCPClientFactory.NewResourceProvidersClient().Get(ctx, "local", r.ResourceProvider.Name, nil)
_, err := r.UCPClientFactory.NewResourceProvidersClient().Get(ctx, "local", r.ResourceProvider.Name, nil)
if err != nil {
// 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) {
if clients.Is404Error(err) {
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
Expand All @@ -160,9 +157,22 @@ func (r *Runner) Run(ctx context.Context) error {
}
}

_, err = r.UCPClientFactory.NewResourceTypesClient().Get(ctx, "local", r.ResourceProvider.Name, r.ResourceTypeName, nil)
if err != nil {
return err
}

r.Output.LogInfo("")

err = r.Output.WriteFormatted(r.Format, response, common.GetResourceTypeTableFormat())
client, err := r.ConnectionFactory.CreateApplicationsManagementClient(ctx, *r.Workspace)
if err != nil {
return err
}
resourceTypeDetails, err := common.GetResourceTypeDetails(ctx, r.ResourceProvider.Name, r.ResourceTypeName, client)
if err != nil {
return err
}
err = r.Output.WriteFormatted(r.Format, resourceTypeDetails, common.GetResourceTypeTableFormat())
if err != nil {
return err
}
Expand Down
Loading

0 comments on commit 846f546

Please sign in to comment.