Skip to content

Commit

Permalink
Fixing Gateways TLS minimumProtocolVersion bug (radius-project#5683)
Browse files Browse the repository at this point in the history
# Description

* Fixing a bug where not specifying TLS minumumProtocolVersion would
cause deployments to fail. It should default to `1.2`

## Issue reference

<!--
We strive to have all PR being opened based on an issue, where the
problem or feature have been discussed prior to implementation.
-->

Fixes: radius-project#5682 

## Checklist

Please make sure you've completed the relevant tasks for this PR, out of
the following list:

* [ ] Code compiles correctly
* [ ] Adds necessary unit tests for change
* [ ] Adds necessary E2E tests for change
* [ ] Unit tests passing
* [ ] Extended the documentation / Created issue for it

## Auto-generated summary

<!--
GitHub Copilot for docs will auto-generate a summary of the PR
-->

<!--
copilot:all
-->
### <samp>🤖 Generated by Copilot at 0b8569f</samp>

### Summary
📄🧪🐛

<!--
1.  📄 for adding and modifying files in the `testdata` folder.
2. 🧪 for adding and updating tests and test fixtures in the
`gateway_conversion_test.go` file.
3. 🐛 for fixing a bug and removing unused code in the
`gateway_conversion.go` file.
-->
This pull request updates the gateway conversion and testing code to
handle the changes in the versioned API model and the data model for the
gateway resource with TLS termination. It also fixes a bug in the
conversion logic and adds test cases for the scenario when the minimum
protocol version is not specified. It also modifies the bicep template
for deploying a gateway resource with TLS termination to remove the
`minimumProtocolVersion` property.

> _`gateway` resource_
> _TLS conversion fixed_
> _autumn bug hunting_

### Walkthrough
* Remove unused import statement from `gateway_conversion.go`
([link](https://github.com/project-radius/radius/pull/5683/files?diff=unified&w=0#diff-1b1effc4610d0933423711d51cfe96a6835aa5cc54cc922a0f5f3acb1e858c3fL28))
* Add nil check and default value for TLS minimum protocol version
conversion in `gateway_conversion.go`
([link](https://github.com/project-radius/radius/pull/5683/files?diff=unified&w=0#diff-1b1effc4610d0933423711d51cfe96a6835aa5cc54cc922a0f5f3acb1e858c3fL154-R163))
* Update expected TLS minimum protocol version from 1.2 to 1.3 in
`gateway_conversion_test.go` and test fixture files
([link](https://github.com/project-radius/radius/pull/5683/files?diff=unified&w=0#diff-cea35ff8fa843c923eab085ec2345fa1109868714214ff009efd93c832dfaa14L166-R166),
[link](https://github.com/project-radius/radius/pull/5683/files?diff=unified&w=0#diff-bd441cfd96ffeb8421c84f3b346d283b7d43995f296cc986503b5fa751974ba2L31-R31),
[link](https://github.com/project-radius/radius/pull/5683/files?diff=unified&w=0#diff-e68be033427ad045d11d1beb15409cf3a2230eb3cf6c753592868bb609d6db5bL42-R42))
* Add new test functions and fixture files for TLS minimum protocol
version conversion when not specified in `gateway_conversion_test.go`
([link](https://github.com/project-radius/radius/pull/5683/files?diff=unified&w=0#diff-cea35ff8fa843c923eab085ec2345fa1109868714214ff009efd93c832dfaa14R195-R252),
[link](https://github.com/project-radius/radius/pull/5683/files?diff=unified&w=0#diff-ae6b4a2f16cf22d0cc0de932d17ebe284142eff9d31f30e999674cd8fb5be010L1-R33),
[link](https://github.com/project-radius/radius/pull/5683/files?diff=unified&w=0#diff-79809ffd8c8a9ebacab2dd81a72546337d6be933b59c58c5e30d9ce71f441eb6L1-R44))
* Remove `minimumProtocolVersion` property from bicep template for
gateway deployment in `corerp-resources-gateway-tlstermination.bicep`
([link](https://github.com/project-radius/radius/pull/5683/files?diff=unified&w=0#diff-eef570133c134137d3c5b556de0fe53a586ae1dd477e5d9cdd46e21948c618f6L29-R29))

---------

Co-authored-by: Young Bu Park <youngp@microsoft.com>
  • Loading branch information
willdavsmith and youngbupark authored Jun 20, 2023
1 parent cf7e8ef commit 7875c92
Show file tree
Hide file tree
Showing 8 changed files with 148 additions and 7 deletions.
7 changes: 5 additions & 2 deletions pkg/corerp/api/v20220315privatepreview/gateway_conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (

// ConvertTo converts from the versioned Gateway resource to version-agnostic datamodel.
func (src *GatewayResource) ConvertTo() (v1.DataModelInterface, error) {

tls := &datamodel.GatewayPropertiesTLS{}
if src.Properties.TLS == nil {
tls = nil
Expand Down Expand Up @@ -149,13 +148,17 @@ func (dst *GatewayResource) ConvertFrom(src v1.DataModelInterface) error {
}

func toTLSMinVersionDataModel(tlsMinVersion *TLSMinVersion) datamodel.MinimumTLSProtocolVersion {
if tlsMinVersion == nil {
return datamodel.DefaultTLSMinVersion
}

switch *tlsMinVersion {
case TLSMinVersionOne2:
return datamodel.TLSMinVersion12
case TLSMinVersionOne3:
return datamodel.TLSMinVersion13
default:
return datamodel.TLSMinVersion12
return datamodel.DefaultTLSMinVersion
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ func TestGatewayTLSTerminationConvertVersionedToDataModel(t *testing.T) {
require.Equal(t, []rpv1.OutputResource(nil), gw.Properties.Status.OutputResources)
require.Equal(t, "2022-03-15-privatepreview", gw.InternalMetadata.UpdatedAPIVersion)
require.Equal(t, "secretname", gw.Properties.TLS.CertificateFrom)
require.Equal(t, datamodel.TLSMinVersion12, gw.Properties.TLS.MinimumProtocolVersion)
require.Equal(t, datamodel.TLSMinVersion13, gw.Properties.TLS.MinimumProtocolVersion)
}

func TestGatewayTLSTerminationConvertDataModelToVersioned(t *testing.T) {
Expand All @@ -177,6 +177,64 @@ func TestGatewayTLSTerminationConvertDataModelToVersioned(t *testing.T) {
versioned := &GatewayResource{}
err = versioned.ConvertFrom(r)

// assert
require.NoError(t, err)
require.Equal(t, "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/radius-test-rg/providers/Applications.Core/gateways/gateway0", *versioned.ID)
require.Equal(t, "gateway0", *versioned.Name)
require.Equal(t, "Applications.Core/gateways", *versioned.Type)
require.Equal(t, "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/testGroup/providers/Applications.Core/applications/app0", *versioned.Properties.Application)
require.Equal(t, "myapp.mydomain.com", *versioned.Properties.Hostname.FullyQualifiedHostname)
require.Equal(t, "myprefix", *versioned.Properties.Hostname.Prefix)
require.Equal(t, "myreplaceprefix", *versioned.Properties.Routes[0].ReplacePrefix)
require.Equal(t, "mypath", *versioned.Properties.Routes[0].Path)
require.Equal(t, "myreplaceprefix", *versioned.Properties.Routes[0].ReplacePrefix)
require.Equal(t, "http://myprefix.myapp.mydomain.com", *versioned.Properties.URL)
require.Equal(t, "Deployment", versioned.Properties.Status.OutputResources[0]["LocalID"])
require.Equal(t, "kubernetes", versioned.Properties.Status.OutputResources[0]["Provider"])
require.Equal(t, "secretname", *versioned.Properties.TLS.CertificateFrom)
require.Equal(t, TLSMinVersionOne3, *versioned.Properties.TLS.MinimumProtocolVersion)
}

func TestGatewayTLSTerminationConvertVersionedToDataModel_NoMinProtocolVersion(t *testing.T) {
// arrange
rawPayload := testutil.ReadFixture("gatewayresource-with-tlstermination-nominprotocolversion.json")
r := &GatewayResource{}
err := json.Unmarshal(rawPayload, r)
require.NoError(t, err)

// act
dm, err := r.ConvertTo()

// assert
require.NoError(t, err)
gw := dm.(*datamodel.Gateway)
require.Equal(t, "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/radius-test-rg/providers/Applications.Core/gateways/gateway0", gw.ID)
require.Equal(t, "gateway0", gw.Name)
require.Equal(t, "Applications.Core/gateways", gw.Type)
require.Equal(t, "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/testGroup/providers/Applications.Core/applications/app0", gw.Properties.Application)
require.Equal(t, "myapp.mydomain.com", gw.Properties.Hostname.FullyQualifiedHostname)
require.Equal(t, "myprefix", gw.Properties.Hostname.Prefix)
require.Equal(t, "mydestination", gw.Properties.Routes[0].Destination)
require.Equal(t, "mypath", gw.Properties.Routes[0].Path)
require.Equal(t, "myreplaceprefix", gw.Properties.Routes[0].ReplacePrefix)
require.Equal(t, "http://myprefix.myapp.mydomain.com", gw.Properties.URL)
require.Equal(t, []rpv1.OutputResource(nil), gw.Properties.Status.OutputResources)
require.Equal(t, "2022-03-15-privatepreview", gw.InternalMetadata.UpdatedAPIVersion)
require.Equal(t, "secretname", gw.Properties.TLS.CertificateFrom)
require.Equal(t, datamodel.DefaultTLSMinVersion, gw.Properties.TLS.MinimumProtocolVersion)
}

func TestGatewayTLSTerminationConvertDataModelToVersioned_NoMinProtocolVersion(t *testing.T) {
// arrange
rawPayload := testutil.ReadFixture("gatewayresourcedatamodel-with-tlstermination-nominprotocolversion.json")
r := &datamodel.Gateway{}
err := json.Unmarshal(rawPayload, r)
require.NoError(t, err)

// act
versioned := &GatewayResource{}
err = versioned.ConvertFrom(r)

// assert
require.NoError(t, err)
require.Equal(t, "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/radius-test-rg/providers/Applications.Core/gateways/gateway0", *versioned.ID)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
{
"id": "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/radius-test-rg/providers/Applications.Core/gateways/gateway0",
"name": "gateway0",
"type": "Applications.Core/gateways",
"properties": {
"status": {
"outputResources": [
{
"LocalID": "Deployment",
"ResourceType": {
"Type": "Gateway",
"Provider": "kubernetes"
}
}
]
},
"application": "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/testGroup/providers/Applications.Core/applications/app0",
"hostname": {
"fullyQualifiedHostname": "myapp.mydomain.com",
"prefix": "myprefix"
},
"routes": [
{
"destination": "mydestination",
"path": "mypath",
"replacePrefix": "myreplaceprefix"
}
],
"tls": {
"certificateFrom": "secretname"
},
"url": "http://myprefix.myapp.mydomain.com"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
],
"tls": {
"certificateFrom": "secretname",
"minimumProtocolVersion": "1.2"
"minimumProtocolVersion": "1.3"
},
"url": "http://myprefix.myapp.mydomain.com"
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
{
"id": "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/radius-test-rg/providers/Applications.Core/gateways/gateway0",
"name": "gateway0",
"type": "Applications.Core/gateways",
"systemData": {
"createdBy": "fakeid@live.com",
"createdByType": "User",
"createdAt": "2021-09-24T19:09:54.2403864Z",
"lastModifiedBy": "fakeid@live.com",
"lastModifiedByType": "User",
"lastModifiedAt": "2021-09-24T20:09:54.2403864Z"
},
"tags": {
"env": "dev"
},
"properties": {
"status": {
"outputResources": [
{
"LocalID": "Deployment",
"ResourceType": {
"Type": "Gateway",
"Provider": "kubernetes"
}
}
]
},
"application": "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/testGroup/providers/Applications.Core/applications/app0",
"hostname": {
"fullyQualifiedHostname": "myapp.mydomain.com",
"prefix": "myprefix"
},
"routes": [
{
"destination": "mydestination",
"path": "mypath",
"replacePrefix": "myreplaceprefix"
}
],
"tls": {
"certificateFrom": "secretname"
},
"url": "http://myprefix.myapp.mydomain.com"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
],
"tls": {
"certificateFrom": "secretname",
"minimumProtocolVersion": "1.2"
"minimumProtocolVersion": "1.3"
},
"url": "http://myprefix.myapp.mydomain.com"
}
Expand Down
2 changes: 2 additions & 0 deletions pkg/corerp/datamodel/gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,8 @@ const (
TLSMinVersion12 MinimumTLSProtocolVersion = "1.2"
// TLS 1.3
TLSMinVersion13 MinimumTLSProtocolVersion = "1.3"
// Default is TLS 1.2
DefaultTLSMinVersion MinimumTLSProtocolVersion = TLSMinVersion12
)

// ValidMinimumTLSProtocolVersions returns a list of valid MinimumTLSProtocolVersions.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@ resource gateway 'Applications.Core/gateways@2022-03-15-privatepreview' = {
name: 'tls-gtwy-gtwy'
properties: {
application: app.id
tls: {
minimumProtocolVersion: '1.2'
tls: {
certificateFrom: certificate.id
}
routes: [
Expand Down

0 comments on commit 7875c92

Please sign in to comment.