Skip to content

Conversation

@sk593
Copy link
Contributor

@sk593 sk593 commented Sep 15, 2025

This PR adds Recipes for the Routes resource type. Since Routes need a container for the connection to work, this PR also adds a simple container Recipe

@sk593 sk593 force-pushed the add-routes-recipes branch 7 times, most recently from 92aa70e to de46894 Compare September 15, 2025 22:30
# Define common configuration
setup_config() {
resource_folders=("Security")
# Define specific resource types to test instead of entire folders
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updating the workflow to allow testing to be added for individual resources instead of a namespace

@sk593 sk593 force-pushed the add-routes-recipes branch 3 times, most recently from 26f04ab to 801d78a Compare September 15, 2025 23:58
}
}

resource myContainer 'Radius.Compute/containers@2025-08-01-preview' = {
Copy link
Contributor Author

@sk593 sk593 Sep 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this application template as an example of how the routes resource can be deployed. I'm not sure that we'd want this to be a test scenario for the repo since it's more of an integration/E2E test and it'd require the Gateway CRD to be installed on the test cluster (I set up the CRDs and necessary RBAC to deploy this when manually testing to verify that it'd work)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm open to suggestions on if this should be included in this repo's test framework

namespace: context.runtime.kubernetes.namespace
}
]
hostnames: length(hostnames) > 0 ? hostnames : ['localhost']
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than defaulting to localhost, I think this should be not specified if it is not provided. From the docs:

If no hostname is specified, traffic is routed based on HTTPRoute rules and filters (optional).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed this was not updated in the Terraform Recipe, only Bicep.

backendRefs: [
{
name: '${toLower(last(split(rule.destinationContainer.resourceId, '/')))}-${rule.destinationContainer.containerName}'
port: 80
Copy link
Contributor

@zachcasper zachcasper Sep 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs reference rule.destinationContainer.containerPortName. We cannot assume a port number.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed offline. We'll update the containerPortName variable to expect an integer so it can be passed as an accessible input variable

@sk593 sk593 marked this pull request as ready for review September 30, 2025 18:42
@sk593 sk593 requested review from a team as code owners September 30, 2025 18:42
@sk593 sk593 force-pushed the add-routes-recipes branch from c60f4df to 20f2eb0 Compare September 30, 2025 19:21
@lakshmimsft
Copy link
Contributor

application is set to required in the schema.

required: [environment, application, rules]

But it's not required in the recipe. Pls check if we need to update schema to align with recipe.

@lakshmimsft
Copy link
Contributor

@zachcasper , @sk593 perhaps this was discussed already but should include gRPC as another route_kind in the yaml and update the recipe for it?

@sk593 sk593 force-pushed the add-routes-recipes branch from 20f2eb0 to 00cacb0 Compare October 21, 2025 16:55
@sk593
Copy link
Contributor Author

sk593 commented Oct 21, 2025

@zachcasper , @sk593 perhaps this was discussed already but should include gRPC as another route_kind in the yaml and update the recipe for it?

I don't think this was discussed but we can add gRPC. thoughts @zachcasper ?

@YohanSciubukgian
Copy link

Will this support WebSocket connections, including HTTPS-to-WSS protocol switching, especially in the context of route-based promotion capabilities?

@sk593 sk593 force-pushed the add-routes-recipes branch from 00cacb0 to ec78cda Compare October 28, 2025 21:18
@sk593
Copy link
Contributor Author

sk593 commented Oct 28, 2025

Will this support WebSocket connections, including HTTPS-to-WSS protocol switching, especially in the context of route-based promotion capabilities?

I'm not too familiar with HTTPs-to-WSS but I believe it should. Routes support HTTP/HTTPS so the setup for a WebSocket connection would need to be defined in the Recipe. @zachcasper do you have any additional input on this?

@sk593
Copy link
Contributor Author

sk593 commented Oct 28, 2025

application is set to required in the schema.

required: [environment, application, rules]

But it's not required in the recipe. Pls check if we need to update schema to align with recipe.

I don't think it needs to be required by the Recipe. But the resource should be tied to an application


// Assume Gateway already exists - use a default gateway name
// Platform engineers should configure this via recipe parameters or environment
var gatewayName = 'default-gateway'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What gateway controllers use default-gateway such that we should assume a default? I looked at NGINX and the default name is the release name. The Cilium gateway name is cilium.

I'm afraid users will get an error that says "default-gateway gateway not found" instead of "gatewayName parameter not set".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The recipe was being tested using the Kubernetes Gateway API. We can update this to use a parameter instead of a default name


# Assume Gateway already exists - use a default gateway name
# Platform engineers should configure this via recipe parameters or environment
gateway_name = "default-gateway"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See previous comment about 'default-gateway'.

namespace = var.context.runtime.kubernetes.namespace
}
]
hostnames = length(local.hostnames) > 0 ? local.hostnames : ["localhost"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I previously made this comment in the Bicep recipe:

Rather than defaulting to localhost, I think this should be not specified if it is not provided. From the docs:

If no hostname is specified, traffic is routed based on HTTPRoute rules and filters (optional).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, looks like this was updated in the Bicep recipe but not Terraform. Will update

@zachcasper
Copy link
Contributor

application is set to required in the schema.

required: [environment, application, rules]

But it's not required in the recipe. Pls check if we need to update schema to align with recipe.

What would it mean to require application in the Recipe since application is only within Radius? I don't quite understand what you're proposing.

@zachcasper
Copy link
Contributor

Will this support WebSocket connections, including HTTPS-to-WSS protocol switching, especially in the context of route-based promotion capabilities?

I'm not too familiar with HTTPs-to-WSS but I believe it should. Routes support HTTP/HTTPS so the setup for a WebSocket connection would need to be defined in the Recipe. @zachcasper do you have any additional input on this?

That sounds right to me. Should be HTTP route type.

@zachcasper
Copy link
Contributor

@zachcasper , @sk593 perhaps this was discussed already but should include gRPC as another route_kind in the yaml and update the recipe for it?

I don't think this was discussed but we can add gRPC. thoughts @zachcasper ?

gRPC can easily be added in the future. We've covered the majority of cases here.

Signed-off-by: sk593 <shruthikumar@microsoft.com>
Signed-off-by: sk593 <shruthikumar@microsoft.com>
Signed-off-by: sk593 <shruthikumar@microsoft.com>
Signed-off-by: sk593 <shruthikumar@microsoft.com>
Signed-off-by: sk593 <shruthikumar@microsoft.com>
Signed-off-by: sk593 <shruthikumar@microsoft.com>
Signed-off-by: sk593 <shruthikumar@microsoft.com>
@sk593 sk593 force-pushed the add-routes-recipes branch from 2f5ef09 to fb57293 Compare November 19, 2025 17:32
Signed-off-by: sk593 <shruthikumar@microsoft.com>
@sk593 sk593 force-pushed the add-routes-recipes branch from 648770a to 01d5129 Compare November 24, 2025 22:32
Signed-off-by: sk593 <shruthikumar@microsoft.com>
@sk593 sk593 force-pushed the add-routes-recipes branch from 01d5129 to 59ab01d Compare November 24, 2025 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants