Skip to content

Commit

Permalink
Add cli validation for missing Bicep parameters (#7298)
Browse files Browse the repository at this point in the history
# Description

This change adds validation for parameters that required by a Bicep
template but not supplied by the CLI. There are some nasty cases today
that can occur when the application parameter is defined, but not
supplied. Short circuiting this behavior on the CLI improves the
user-experience.

## Type of change

- This pull request fixes a bug in Radius and has an approved issue
(issue link required).

Fixes: #issue_number

Signed-off-by: Ryan Nowak <[email protected]>
  • Loading branch information
rynowak authored Apr 8, 2024
1 parent 551539f commit 0a3d68a
Show file tree
Hide file tree
Showing 7 changed files with 413 additions and 16 deletions.
9 changes: 5 additions & 4 deletions pkg/cli/bicep/envinject.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,13 @@ func InjectApplicationParam(deploymentTemplate map[string]any, parameters map[st
}

func injectParam(deploymentTemplate map[string]any, parameters map[string]map[string]any, parameter string, value string) error {
if deploymentTemplate["parameters"] == nil {
return nil
formalParams, err := ExtractParameters(deploymentTemplate)
if err != nil {
return err
}

innerParameters := deploymentTemplate["parameters"].(map[string]any)
if innerParameters[parameter] == nil {
if _, ok := formalParams[parameter]; !ok {
// If we got here, it means 'parameter' is not a formal parameter of the template.
return nil
}

Expand Down
48 changes: 48 additions & 0 deletions pkg/cli/bicep/parameters.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
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 bicep

import "fmt"

// ExtractParameters extracts the parameters from the deployment template.
func ExtractParameters(template map[string]any) (map[string]any, error) {
if template["parameters"] == nil {
return map[string]any{}, nil
}

params, ok := template["parameters"].(map[string]any)
if !ok {
return nil, fmt.Errorf("invalid template: parameters must be a map of maps, got: %T", template["parameters"])
}

return params, nil
}

// DefaultValue returns the default value of a parameter and a boolean indicating if it was found.
func DefaultValue(parameter any) (any, bool) {
if parameter == nil {
return nil, false
}

param, ok := parameter.(map[string]any)
if !ok {
return nil, false
}

defaultValue, ok := param["defaultValue"]
return defaultValue, ok
}
81 changes: 81 additions & 0 deletions pkg/cli/bicep/parameters_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
/*
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 bicep

import (
"encoding/json"
"os"
"testing"

"github.com/stretchr/testify/require"
)

func Test_ExtractParameters(t *testing.T) {
b, err := os.ReadFile("testdata/test-extractparameters.json")
require.NoError(t, err)

template := map[string]any{}
err = json.Unmarshal(b, &template)
require.NoError(t, err)

params, err := ExtractParameters(template)
require.NoError(t, err)

expected := map[string]any{
"application": map[string]any{
"metadata": map[string]any{
"description": "Specifies the application for resources.",
},
"type": "string",
},
"location": map[string]any{
"defaultValue": "westus2",
"metadata": map[string]any{
"description": "Specifies the location for resources.",
},
"type": "string",
},
}
require.Equal(t, expected, params)
}

func Test_DefaultValue_HasDefaultValue(t *testing.T) {
parameter := map[string]any{
"defaultValue": "westus2",
"metadata": map[string]any{
"description": "Specifies the location for resources.",
},
"type": "string",
}

value, ok := DefaultValue(parameter)
require.Equal(t, "westus2", value)
require.True(t, ok)
}

func Test_DefaultValue_NoDefaultValue(t *testing.T) {
parameter := map[string]any{
"metadata": map[string]any{
"description": "Specifies the location for resources.",
},
"type": "string",
}

value, ok := DefaultValue(parameter)
require.Nil(t, value)
require.False(t, ok)
}
24 changes: 24 additions & 0 deletions pkg/cli/bicep/testdata/test-extractparameters.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
{
"$schema": "https://schema.management.azure.com/schemas/2019-04-01/deploymentTemplate.json#",
"languageVersion": "1.9-experimental",
"contentVersion": "1.0.0.0",
"metadata": {
},
"parameters": {
"location": {
"type": "string",
"defaultValue": "westus2",
"metadata": {
"description": "Specifies the location for resources."
}
},
"application": {
"type": "string",
"metadata": {
"description": "Specifies the application for resources."
}
}
},
"resources": {
}
}
87 changes: 87 additions & 0 deletions pkg/cli/cmd/deploy/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ package deploy
import (
"context"
"fmt"
"sort"
"strings"

v1 "github.com/radius-project/radius/pkg/armrpc/api/v1"
"github.com/radius-project/radius/pkg/cli"
Expand All @@ -34,6 +36,7 @@ import (
"github.com/radius-project/radius/pkg/corerp/api/v20231001preview"
"github.com/radius-project/radius/pkg/to"
"github.com/spf13/cobra"
"golang.org/x/exp/maps"
)

// NewCommand creates an instance of the command and runner for the `rad deploy` command.
Expand Down Expand Up @@ -246,6 +249,20 @@ func (r *Runner) Run(ctx context.Context) error {
return err
}

// This is the earliest point where we can inject parameters, we have
// to wait until the template is prepared.
err = r.injectAutomaticParameters(template)
if err != nil {
return err
}

// This is the earliest point where we can report missing parameters, we have
// to wait until the template is prepared.
err = r.reportMissingParameters(template)
if err != nil {
return err
}

// Create application if specified. This supports the case where the application resource
// is not specified in Bicep. Creating the application automatically helps us "bootstrap" in a new environment.
if r.ApplicationName != "" {
Expand Down Expand Up @@ -303,3 +320,73 @@ func (r *Runner) Run(ctx context.Context) error {

return nil
}

func (r *Runner) injectAutomaticParameters(template map[string]any) error {
if r.Providers.Radius.EnvironmentID != "" {
err := bicep.InjectEnvironmentParam(template, r.Parameters, r.Providers.Radius.EnvironmentID)
if err != nil {
return err
}
}

if r.Providers.Radius.ApplicationID != "" {
err := bicep.InjectApplicationParam(template, r.Parameters, r.Providers.Radius.ApplicationID)
if err != nil {
return err
}
}

return nil
}

func (r *Runner) reportMissingParameters(template map[string]any) error {
declaredParameters, err := bicep.ExtractParameters(template)
if err != nil {
return err
}

errors := map[string]string{}
for parameter := range declaredParameters {
// Case-invariant lookup on the user-provided values
match := false
for provided := range r.Parameters {
if strings.EqualFold(parameter, provided) {
match = true
break
}
}

if match {
// Has user-provided value
continue
}

if _, ok := bicep.DefaultValue(declaredParameters[parameter]); ok {
// Has default value
continue
}

// Special case the parameters that are automatically injected
if strings.EqualFold(parameter, "environment") {
errors[parameter] = "The template requires an environment. Use --environment to specify the environment name."
} else if strings.EqualFold(parameter, "application") {
errors[parameter] = "The template requires an application. Use --application to specify the application name."
} else {
errors[parameter] = fmt.Sprintf("The template requires a parameter %q. Use --parameters %s=<value> to specify the value.", parameter, parameter)
}
}

if len(errors) == 0 {
return nil
}

keys := maps.Keys(errors)
sort.Strings(keys)

details := []string{}
for _, key := range keys {
details = append(details, fmt.Sprintf(" - %v", errors[key]))
}

return clierrors.Message("The template %q could not be deployed because of the following errors:\n\n%v", r.FilePath, strings.Join(details, "\n"))
}
Loading

0 comments on commit 0a3d68a

Please sign in to comment.