Add terraform template path validation #6969

sk593 · 2023-12-18T23:45:38Z

Description

This adds validation for terraform template paths. We will only support Terraform registry and HTTP URLs as allowed module sources for Terraform recipe template paths. Module sources outside these will be treated as unsupported and an error will be returned.

Type of change

This pull request adds or changes features of Radius and has an approved issue Validation for Terraform Recipe Template Paths #6642

Fixes: #6642

Auto-generated summary

copilot:all

radius-functional-tests · 2023-12-19T00:11:18Z

Radius functional test overview

🔍 Go to test action run

Name	Value
Repository	sk593/radius
Commit ref	`ff53c81`
Unique ID	23901fe398
Image tag	pr-23901fe398

Click here to see the list of tools in the current test run

gotestsum 1.10.0
KinD: v0.20.0
Dapr: 1.12.0
Azure KeyVault CSI driver: 1.4.2
Azure Workload identity webhook: 1.1.0
Bicep recipe location ghcr.io/radius-project/dev/test/functional/shared/recipes/<name>:pr-23901fe398
Terraform recipe location http://tf-module-server.radius-test-tf-module-server.svc.cluster.local/<name>.zip (in cluster)
applications-rp test image location: ghcr.io/radius-project/dev/applications-rp:pr-23901fe398
controller test image location: ghcr.io/radius-project/dev/controller:pr-23901fe398
ucp test image location: ghcr.io/radius-project/dev/ucpd:pr-23901fe398
deployment-engine test image location: ghcr.io/radius-project/deployment-engine:latest

Test Status

⌛ Building Radius and pushing container images for functional tests...
✅ Container images build succeeded
⌛ Publishing Bicep Recipes for functional tests...
✅ Recipe publishing succeeded
⌛ Starting datastoresrp functional tests...
⌛ Starting shared functional tests...
⌛ Starting ucp functional tests...
⌛ Starting msgrp functional tests...
⌛ Starting daprrp functional tests...
⌛ Starting samples functional tests...
⌛ Starting kubernetes functional tests...
✅ ucp functional tests succeeded
✅ msgrp functional tests succeeded
✅ samples functional tests succeeded
✅ datastoresrp functional tests succeeded
✅ kubernetes functional tests succeeded
✅ daprrp functional tests succeeded
✅ shared functional tests succeeded

radius-functional-tests · 2024-01-04T19:22:09Z

Radius functional test overview

🔍 Go to test action run

Name	Value
Repository	sk593/radius
Commit ref	`8cd781b`
Unique ID	af06849c7c
Image tag	pr-af06849c7c

Click here to see the list of tools in the current test run

gotestsum 1.10.0
KinD: v0.20.0
Dapr: 1.12.0
Azure KeyVault CSI driver: 1.4.2
Azure Workload identity webhook: 1.1.0
Bicep recipe location ghcr.io/radius-project/dev/test/functional/shared/recipes/<name>:pr-af06849c7c
Terraform recipe location http://tf-module-server.radius-test-tf-module-server.svc.cluster.local/<name>.zip (in cluster)
applications-rp test image location: ghcr.io/radius-project/dev/applications-rp:pr-af06849c7c
controller test image location: ghcr.io/radius-project/dev/controller:pr-af06849c7c
ucp test image location: ghcr.io/radius-project/dev/ucpd:pr-af06849c7c
deployment-engine test image location: ghcr.io/radius-project/deployment-engine:latest

Test Status

⌛ Building Radius and pushing container images for functional tests...
✅ Container images build succeeded
⌛ Publishing Bicep Recipes for functional tests...
✅ Recipe publishing succeeded
⌛ Starting shared functional tests...
⌛ Starting kubernetes functional tests...
⌛ Starting ucp functional tests...
⌛ Starting datastoresrp functional tests...
⌛ Starting samples functional tests...
⌛ Starting daprrp functional tests...
⌛ Starting msgrp functional tests...
✅ ucp functional tests succeeded
✅ msgrp functional tests succeeded
✅ kubernetes functional tests succeeded
✅ samples functional tests succeeded
✅ datastoresrp functional tests succeeded
✅ daprrp functional tests succeeded
✅ shared functional tests succeeded

pkg/corerp/api/v20231001preview/environment_conversion.go

pkg/corerp/api/v20231001preview/environment_conversion_test.go

radius-functional-tests · 2024-01-08T19:07:14Z

Radius functional test overview

🔍 Go to test action run

Name	Value
Repository	sk593/radius
Commit ref	`9b47d5e`
Unique ID	61617d68a5
Image tag	pr-61617d68a5

Click here to see the list of tools in the current test run

gotestsum 1.10.0
KinD: v0.20.0
Dapr: 1.12.0
Azure KeyVault CSI driver: 1.4.2
Azure Workload identity webhook: 1.1.0
Bicep recipe location ghcr.io/radius-project/dev/test/functional/shared/recipes/<name>:pr-61617d68a5
Terraform recipe location http://tf-module-server.radius-test-tf-module-server.svc.cluster.local/<name>.zip (in cluster)
applications-rp test image location: ghcr.io/radius-project/dev/applications-rp:pr-61617d68a5
controller test image location: ghcr.io/radius-project/dev/controller:pr-61617d68a5
ucp test image location: ghcr.io/radius-project/dev/ucpd:pr-61617d68a5
deployment-engine test image location: ghcr.io/radius-project/deployment-engine:latest

Test Status

⌛ Building Radius and pushing container images for functional tests...
✅ Container images build succeeded
⌛ Publishing Bicep Recipes for functional tests...
✅ Recipe publishing succeeded
⌛ Starting daprrp functional tests...
⌛ Starting datastoresrp functional tests...
⌛ Starting ucp functional tests...
⌛ Starting shared functional tests...
⌛ Starting msgrp functional tests...
⌛ Starting samples functional tests...
⌛ Starting kubernetes functional tests...
✅ samples functional tests succeeded
✅ msgrp functional tests succeeded
✅ kubernetes functional tests succeeded
✅ datastoresrp functional tests succeeded
✅ ucp functional tests succeeded
✅ daprrp functional tests succeeded
✅ shared functional tests succeeded

radius-functional-tests · 2024-01-09T18:21:15Z

Radius functional test overview

🔍 Go to test action run

Name	Value
Repository	sk593/radius
Commit ref	`50ddbc0`
Unique ID	0b939f226c
Image tag	pr-0b939f226c

Click here to see the list of tools in the current test run

gotestsum 1.10.0
KinD: v0.20.0
Dapr: 1.12.0
Azure KeyVault CSI driver: 1.4.2
Azure Workload identity webhook: 1.1.0
Bicep recipe location ghcr.io/radius-project/dev/test/functional/shared/recipes/<name>:pr-0b939f226c
Terraform recipe location http://tf-module-server.radius-test-tf-module-server.svc.cluster.local/<name>.zip (in cluster)
applications-rp test image location: ghcr.io/radius-project/dev/applications-rp:pr-0b939f226c
controller test image location: ghcr.io/radius-project/dev/controller:pr-0b939f226c
ucp test image location: ghcr.io/radius-project/dev/ucpd:pr-0b939f226c
deployment-engine test image location: ghcr.io/radius-project/deployment-engine:latest

Test Status

⌛ Building Radius and pushing container images for functional tests...
✅ Container images build succeeded
⌛ Publishing Bicep Recipes for functional tests...
✅ Recipe publishing succeeded
⌛ Starting shared functional tests...
⌛ Starting ucp functional tests...
⌛ Starting daprrp functional tests...
⌛ Starting msgrp functional tests...
⌛ Starting samples functional tests...
⌛ Starting kubernetes functional tests...
⌛ Starting datastoresrp functional tests...
✅ msgrp functional tests succeeded
✅ samples functional tests succeeded
✅ kubernetes functional tests succeeded
✅ daprrp functional tests succeeded
✅ ucp functional tests succeeded
✅ datastoresrp functional tests succeeded
✅ shared functional tests succeeded

ytimocin · 2024-01-09T18:23:51Z

pkg/corerp/api/v20231001preview/environment_conversion_test.go

+			err:      &v1.ErrClientRP{Code: v1.CodeInvalid, Message: fmt.Sprintf(invalidModulePathFmt, "../not-allowed/")},
+		},
+		{
+			filename: "environmentresource-terraformrecipe-unsupported.json",


This will be denied by the ParseRequestURI and the other one by the ParseModuleSource, right?

They should both error out since neither of these are a TF registry or an HTTP URL

pkg/corerp/api/v20231001preview/testdata/environmentresource-terraformrecipe-unsupported.json

ytimocin · 2024-01-09T18:24:30Z

pkg/corerp/api/v20231001preview/environment_conversion.go

+			// We first validate if the template path is a valid Terraform registry module source.
+			_, err := tfaddr.ParseModuleSource(to.String(c.TemplatePath))
+			if err != nil {
+				// If the template path is not a Terraform registry module source, we validate if it is an HTTP URL.
+				_, err = url.ParseRequestURI(to.String(c.TemplatePath))
+				if err != nil {
+					return datamodel.EnvironmentRecipeProperties{}, v1.NewClientErrInvalidRequest(fmt.Sprintf(invalidModulePathFmt, to.String(c.TemplatePath)))
+				}


Should this be a separate function? Is this going to be used elsewhere in the code?

No, this is just going to be used here when we try to register a recipe to an environment. We don't want to deploy the resource/recipe at all if we know the module source isn't supported and this is the first entry point for registering the recipe

ytimocin · 2024-01-09T18:24:55Z

pkg/corerp/api/v20231001preview/environment_conversion.go

 	rp_util "github.com/radius-project/radius/pkg/rp/portableresources"
 	rpv1 "github.com/radius-project/radius/pkg/rp/v1"
 	"github.com/radius-project/radius/pkg/to"
 )

 const (
 	EnvironmentComputeKindKubernetes = "kubernetes"
-	invalidLocalModulePathFmt        = "local module paths are not supported with Terraform Recipes. The 'templatePath' '%s' was detected as a local module path because it begins with '/' or './' or '../'."
+	invalidModulePathFmt             = "only Terraform registry and HTTP URLs are supported as module sources with Terraform Recipes. The 'templatePath' '%s' was detected as an invalid module source."


cc/ @AaronCrawfis

AaronCrawfis

Given we verified that git:/ works correctly today, we should probably also support that. I believe @rynowak uses this today as part of AWS demos.

sk593 · 2024-01-09T22:06:55Z

Given we verified that git:/ works correctly today, we should probably also support that. I believe @rynowak uses this today as part of AWS demos.

Git repositories are considered valid HTTP URLs by ParseRequestURI so those should still work. I added a test case for this to confirm. Github URLs, however, are not supported with the current implementation. Do we want to include support for that as well or should we leave it at git:/? cc/ @AaronCrawfis

Signed-off-by: sk593 <[email protected]>

AaronCrawfis · 2024-01-09T22:15:22Z

Git repositories are considered valid HTTP URLs by ParseRequestURI so those should still work. I added a test case for this to confirm. Github URLs, however, are not supported with the current implementation. Do we want to include support for that as well or should we leave it at git:/? cc/ @AaronCrawfis

Hmm good question. Let's also add GitHub as well, as it's essentially just an alias for git. It should work as well. Thanks!

sk593 · 2024-01-11T17:10:47Z

Closing, we thought the initial issue was caused by Radius not supporting all module sources. This doesn't seem to be the case as Git, S3, etc are able to be used so there's no need for additional validation.

sk593 requested review from a team as code owners December 18, 2023 23:45

sk593 force-pushed the validate-tf-paths branch from ff53c81 to 2777c07 Compare December 18, 2023 23:46

sk593 temporarily deployed to functional-tests January 4, 2024 19:21 — with GitHub Actions Inactive

kachawla reviewed Jan 8, 2024

View reviewed changes

pkg/corerp/api/v20231001preview/environment_conversion.go Outdated Show resolved Hide resolved

pkg/corerp/api/v20231001preview/environment_conversion_test.go Outdated Show resolved Hide resolved

pkg/corerp/api/v20231001preview/environment_conversion_test.go Outdated Show resolved Hide resolved

sk593 temporarily deployed to functional-tests January 8, 2024 19:06 — with GitHub Actions Inactive

sk593 force-pushed the validate-tf-paths branch from 9b47d5e to 50ddbc0 Compare January 8, 2024 23:25

kachawla previously approved these changes Jan 9, 2024

View reviewed changes

sk593 temporarily deployed to functional-tests January 9, 2024 18:20 — with GitHub Actions Inactive

ytimocin reviewed Jan 9, 2024

View reviewed changes

shalabhms previously approved these changes Jan 9, 2024

View reviewed changes

sk593 dismissed stale reviews from shalabhms and kachawla via f451e1d January 9, 2024 21:41

sk593 force-pushed the validate-tf-paths branch from 50ddbc0 to f451e1d Compare January 9, 2024 21:41

AaronCrawfis suggested changes Jan 9, 2024

View reviewed changes

adding template path validation

8e35415

Signed-off-by: sk593 <[email protected]>

sk593 force-pushed the validate-tf-paths branch from f451e1d to 8e35415 Compare January 9, 2024 22:10

sk593 closed this Jan 11, 2024

sk593 had a problem deploying to functional-tests February 8, 2024 22:10 — with GitHub Actions Failure

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add terraform template path validation #6969

Add terraform template path validation #6969

sk593 commented Dec 18, 2023

radius-functional-tests bot commented Dec 19, 2023 •

edited

Loading

radius-functional-tests bot commented Jan 4, 2024 •

edited

Loading

radius-functional-tests bot commented Jan 8, 2024 •

edited

Loading

radius-functional-tests bot commented Jan 9, 2024 •

edited

Loading

ytimocin Jan 9, 2024

sk593 Jan 9, 2024

ytimocin Jan 9, 2024

sk593 Jan 9, 2024

ytimocin Jan 9, 2024

AaronCrawfis left a comment

sk593 commented Jan 9, 2024 •

edited

Loading

AaronCrawfis commented Jan 9, 2024

sk593 commented Jan 11, 2024

Add terraform template path validation #6969

Add terraform template path validation #6969

Conversation

sk593 commented Dec 18, 2023

Description

Type of change

Auto-generated summary

radius-functional-tests bot commented Dec 19, 2023 • edited Loading

Radius functional test overview

Test Status

radius-functional-tests bot commented Jan 4, 2024 • edited Loading

Radius functional test overview

Test Status

radius-functional-tests bot commented Jan 8, 2024 • edited Loading

Radius functional test overview

Test Status

radius-functional-tests bot commented Jan 9, 2024 • edited Loading

Radius functional test overview

Test Status

ytimocin Jan 9, 2024

Choose a reason for hiding this comment

sk593 Jan 9, 2024

Choose a reason for hiding this comment

ytimocin Jan 9, 2024

Choose a reason for hiding this comment

sk593 Jan 9, 2024

Choose a reason for hiding this comment

ytimocin Jan 9, 2024

Choose a reason for hiding this comment

AaronCrawfis left a comment

Choose a reason for hiding this comment

sk593 commented Jan 9, 2024 • edited Loading

AaronCrawfis commented Jan 9, 2024

sk593 commented Jan 11, 2024

radius-functional-tests bot commented Dec 19, 2023 •

edited

Loading

radius-functional-tests bot commented Jan 4, 2024 •

edited

Loading

radius-functional-tests bot commented Jan 8, 2024 •

edited

Loading

radius-functional-tests bot commented Jan 9, 2024 •

edited

Loading

sk593 commented Jan 9, 2024 •

edited

Loading