Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

add support for filtering public ingress traffic by IP or CIDR #1198

Merged
merged 10 commits into from
Sep 25, 2024
Merged

add support for filtering public ingress traffic by IP or CIDR #1198

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
cd362c3
add support for filtering public ingress traffic by IP or CIDR
nilsgstrabo Sep 23, 2024
61e3ead
add validation of network in RA
nilsgstrabo Sep 24, 2024
345ddcb
handle error returned by mergo.Merge
nilsgstrabo Sep 24, 2024
52fb91a
Merge branch 'master' into feature/suppoert-ingress-ip-filtering
nilsgstrabo Sep 24, 2024
6e940af
update chart version
nilsgstrabo Sep 24, 2024
e8a312f
Update pkg/apis/ingress/ingressannotationprovider.go
nilsgstrabo Sep 24, 2024
0b83ae8
fix spelling
nilsgstrabo Sep 24, 2024
2dfd3c5
add docs comment to new annotation provider
nilsgstrabo Sep 24, 2024
594d99d
simplify function
nilsgstrabo Sep 24, 2024
82bab8d
simplyfy annotation function
nilsgstrabo Sep 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions charts/radix-operator/Chart.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
apiVersion: v2
name: radix-operator
version: 1.40.0
appVersion: 1.60.0
version: 1.41.0
appVersion: 1.61.0
kubeVersion: ">=1.24.0"
description: Radix Operator
keywords:
Expand Down
44 changes: 44 additions & 0 deletions charts/radix-operator/templates/radixapplication.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -687,6 +687,29 @@ spec:
Enabled or disables collection of custom Prometheus metrics.
More info: https://www.radix.equinor.com/references/reference-radix-config/#monitoring
type: boolean
network:
description: Environment specific network settings.
properties:
ingress:
description: Ingress defines settings for ingress
traffic.
properties:
public:
description: Public defines settings for public
traffic.
properties:
allow:
description: |-
Allow defines a list of public IP adresses or CIDRs which are allowed to access the component.
All IP adresses are allowed if this field is empty or not set.
items:
description: IP address or CIDR.
pattern: ^(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)(\/([0-9]|[1-2][0-9]|3[0-2]))?$
type: string
type: array
type: object
type: object
type: object
node:
description: |-
Environment specific GPU requirements for the component.
Expand Down Expand Up @@ -1406,6 +1429,27 @@ spec:
minLength: 1
pattern: ^(([a-z0-9][-a-z0-9]*)?[a-z0-9])?$
type: string
network:
description: Network settings.
properties:
ingress:
description: Ingress defines settings for ingress traffic.
properties:
public:
description: Public defines settings for public traffic.
properties:
allow:
description: |-
Allow defines a list of public IP adresses or CIDRs which are allowed to access the component.
All IP adresses are allowed if this field is empty or not set.
items:
description: IP address or CIDR.
pattern: ^(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)(\/([0-9]|[1-2][0-9]|3[0-2]))?$
type: string
type: array
type: object
type: object
type: object
node:
description: |-
Defines GPU requirements for the component.
Expand Down
54 changes: 54 additions & 0 deletions json-schema/radixapplication.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -665,6 +665,33 @@
"description": "Enabled or disables collection of custom Prometheus metrics.\nMore info: https://www.radix.equinor.com/references/reference-radix-config/#monitoring",
"type": "boolean"
},
"network": {
"description": "Environment specific network settings.",
"properties": {
"ingress": {
"description": "Ingress defines settings for ingress traffic.",
"properties": {
"public": {
"description": "Public defines settings for public traffic.",
"properties": {
"allow": {
"description": "Allow defines a list of public IP adresses or CIDRs which are allowed to access the component.\nAll IP adresses are allowed if this field is empty or not set.",
"items": {
"description": "IP address or CIDR.",
"pattern": "^(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)(\\/([0-9]|[1-2][0-9]|3[0-2]))?$",
"type": "string"
},
"type": "array"
}
},
"type": "object"
}
},
"type": "object"
}
},
"type": "object"
},
"node": {
"description": "Environment specific GPU requirements for the component.\nMore info: https://www.radix.equinor.com/references/reference-radix-config/#node",
"properties": {
Expand Down Expand Up @@ -1394,6 +1421,33 @@
"pattern": "^(([a-z0-9][-a-z0-9]*)?[a-z0-9])?$",
"type": "string"
},
"network": {
"description": "Network settings.",
"properties": {
"ingress": {
"description": "Ingress defines settings for ingress traffic.",
"properties": {
"public": {
"description": "Public defines settings for public traffic.",
"properties": {
"allow": {
"description": "Allow defines a list of public IP adresses or CIDRs which are allowed to access the component.\nAll IP adresses are allowed if this field is empty or not set.",
"items": {
"description": "IP address or CIDR.",
"pattern": "^(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)(\\/([0-9]|[1-2][0-9]|3[0-2]))?$",
"type": "string"
},
"type": "array"
}
},
"type": "object"
}
},
"type": "object"
}
},
"type": "object"
},
"node": {
"description": "Defines GPU requirements for the component.\nMore info: https://www.radix.equinor.com/references/reference-radix-config/#node",
"properties": {
Expand Down
21 changes: 21 additions & 0 deletions pkg/apis/deployment/radixcomponent.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,33 @@ func GetRadixComponentsForEnv(ctx context.Context, radixApplication *radixv1.Rad
if deployComponent.VolumeMounts, err = getRadixCommonComponentVolumeMounts(&radixComponent, environmentSpecificConfig); err != nil {
return nil, err
}
if deployComponent.Network, err = getRadixComponentNetwork(&radixComponent, environmentSpecificConfig); err != nil {
return nil, err
}
deployComponents = append(deployComponents, deployComponent)
}

return deployComponents, nil
}

func getRadixComponentNetwork(component *radixv1.RadixComponent, environmentConfig *radixv1.RadixEnvironmentConfig) (*radixv1.Network, error) {
var dst *radixv1.Network
if component.Network != nil {
dst = component.Network.DeepCopy()
}

if environmentConfig != nil && environmentConfig.Network != nil {
if dst == nil {
dst = &radixv1.Network{}
}
if err := mergo.Merge(dst, environmentConfig.Network, mergo.WithOverride, mergo.WithOverrideEmptySlice); err != nil {
return nil, err
}
}

return dst, nil
}

func getRadixCommonComponentReadOnlyFileSystem(radixComponent radixv1.RadixCommonComponent, environmentSpecificConfig radixv1.RadixCommonEnvironmentConfig) *bool {
if !commonutils.IsNil(environmentSpecificConfig) && environmentSpecificConfig.GetReadOnlyFileSystem() != nil {
return environmentSpecificConfig.GetReadOnlyFileSystem()
Expand Down
70 changes: 70 additions & 0 deletions pkg/apis/deployment/radixcomponent_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1547,6 +1547,76 @@ func Test_GetRadixComponentsForEnv_Runtime_AlwaysUseFromDeployComponentImages(t
}
}

func Test_Test_GetRadixComponentsForEnv_NetworkIngressPublicAllow(t *testing.T) {
tests := map[string]struct {
commonConfig *radixv1.Network
envConfig *radixv1.Network
setEnv bool
expected *radixv1.Network
}{
"nil in common": {
setEnv: false,
expected: nil,
},
"empty in common, env not set": {
commonConfig: &radixv1.Network{},
setEnv: false,
expected: &radixv1.Network{},
},
"empty in common and nil in env": {
commonConfig: &radixv1.Network{},
setEnv: true,
expected: &radixv1.Network{},
},
"empty in common and env": {
commonConfig: &radixv1.Network{},
setEnv: true,
envConfig: &radixv1.Network{},
expected: &radixv1.Network{},
},
"allow set in common, nil in env": {
commonConfig: &radixv1.Network{Ingress: &radixv1.Ingress{Public: &radixv1.IngressPublic{Allow: &[]radixv1.IPOrCIDR{radixv1.IPOrCIDR("commonip1"), radixv1.IPOrCIDR("commonip2")}}}},
setEnv: true,
envConfig: &radixv1.Network{Ingress: &radixv1.Ingress{Public: &radixv1.IngressPublic{}}},
expected: &radixv1.Network{Ingress: &radixv1.Ingress{Public: &radixv1.IngressPublic{Allow: &[]radixv1.IPOrCIDR{radixv1.IPOrCIDR("commonip1"), radixv1.IPOrCIDR("commonip2")}}}},
},
"allow set in common, empty in env": {
commonConfig: &radixv1.Network{Ingress: &radixv1.Ingress{Public: &radixv1.IngressPublic{Allow: &[]radixv1.IPOrCIDR{radixv1.IPOrCIDR("commonip1"), radixv1.IPOrCIDR("commonip2")}}}},
setEnv: true,
envConfig: &radixv1.Network{Ingress: &radixv1.Ingress{Public: &radixv1.IngressPublic{Allow: &[]radixv1.IPOrCIDR{}}}},
expected: &radixv1.Network{Ingress: &radixv1.Ingress{Public: &radixv1.IngressPublic{Allow: &[]radixv1.IPOrCIDR{}}}},
},
"allow set in common and env": {
commonConfig: &radixv1.Network{Ingress: &radixv1.Ingress{Public: &radixv1.IngressPublic{Allow: &[]radixv1.IPOrCIDR{radixv1.IPOrCIDR("commonip1"), radixv1.IPOrCIDR("commonip2")}}}},
setEnv: true,
envConfig: &radixv1.Network{Ingress: &radixv1.Ingress{Public: &radixv1.IngressPublic{Allow: &[]radixv1.IPOrCIDR{radixv1.IPOrCIDR("envip1"), radixv1.IPOrCIDR("envip2")}}}},
expected: &radixv1.Network{Ingress: &radixv1.Ingress{Public: &radixv1.IngressPublic{Allow: &[]radixv1.IPOrCIDR{radixv1.IPOrCIDR("envip1"), radixv1.IPOrCIDR("envip2")}}}},
},
"allow nil in common set in env": {
commonConfig: &radixv1.Network{Ingress: &radixv1.Ingress{Public: &radixv1.IngressPublic{}}},
setEnv: true,
envConfig: &radixv1.Network{Ingress: &radixv1.Ingress{Public: &radixv1.IngressPublic{Allow: &[]radixv1.IPOrCIDR{radixv1.IPOrCIDR("envip1"), radixv1.IPOrCIDR("envip2")}}}},
expected: &radixv1.Network{Ingress: &radixv1.Ingress{Public: &radixv1.IngressPublic{Allow: &[]radixv1.IPOrCIDR{radixv1.IPOrCIDR("envip1"), radixv1.IPOrCIDR("envip2")}}}},
},
}

for testName, test := range tests {
t.Run(testName, func(t *testing.T) {
const envName = "anyenv"
component := utils.AnApplicationComponent().WithName("anycomponent").WithNetwork(test.commonConfig)
if test.setEnv {
component = component.WithEnvironmentConfigs(
utils.AnEnvironmentConfig().WithEnvironment(envName).WithNetwork(test.envConfig),
)
}
ra := utils.ARadixApplication().WithComponents(component).BuildRA()
components, err := GetRadixComponentsForEnv(context.Background(), ra, nil, envName, make(pipeline.DeployComponentImages), make(radixv1.EnvVarsMap), nil)
require.NoError(t, err)
assert.Equal(t, test.expected, components[0].Network)
})
}
}

func convertRadixDeployComponentToNameSet(deployComponents []radixv1.RadixDeployComponent) map[string]bool {
set := make(map[string]bool)
for _, deployComponent := range deployComponents {
Expand Down
1 change: 1 addition & 0 deletions pkg/apis/ingress/ingress.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,5 +115,6 @@ func GetAnnotationProvider(ingressConfiguration IngressConfiguration, certificat
NewIngressConfigurationAnnotationProvider(ingressConfiguration),
NewClientCertificateAnnotationProvider(certificateNamespace),
NewOAuth2AnnotationProvider(oauth2DefaultConfig),
NewIngressPublicAllowListAnnotationProvider(),
}
}
27 changes: 27 additions & 0 deletions pkg/apis/ingress/ingressannotationprovider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,3 +128,30 @@ func (provider *oauth2AnnotationProvider) GetAnnotations(component radixv1.Radix

return annotations, nil
}

// NewIngressPublicAllowListAnnotationProvider provides Ingress annotations for allowing
// only public traffic from IP adresses defined in Network.Ingress.Public.Allow field
nilsgstrabo marked this conversation as resolved.
Show resolved Hide resolved
func NewIngressPublicAllowListAnnotationProvider() AnnotationProvider {
return &ingressPublicAllowListAnnotationProvider{}
}

type ingressPublicAllowListAnnotationProvider struct{}

func (*ingressPublicAllowListAnnotationProvider) GetAnnotations(component radixv1.RadixCommonDeployComponent, _ string) (map[string]string, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing docs comment (or in the interface)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

if network := component.GetNetwork(); network == nil || network.Ingress == nil || network.Ingress.Public == nil || network.Ingress.Public.Allow == nil || len(*network.Ingress.Public.Allow) == 0 {
return nil, nil
}

annotations := make(map[string]string, 1)
var sb strings.Builder

for i, addr := range *component.GetNetwork().Ingress.Public.Allow {
if i > 0 {
sb.WriteString(",")
}
sb.WriteString(string(addr))
}
annotations["nginx.ingress.kubernetes.io/whitelist-source-range"] = sb.String()

return annotations, nil
Copy link
Contributor

Choose a reason for hiding this comment

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

Optional suggestion

Suggested change
annotations := make(map[string]string, 1)
var sb strings.Builder
for i, addr := range *component.GetNetwork().Ingress.Public.Allow {
if i > 0 {
sb.WriteString(",")
}
sb.WriteString(string(addr))
}
annotations["nginx.ingress.kubernetes.io/whitelist-source-range"] = sb.String()
return annotations, nil
addressList := slice.Reduce(*component.GetNetwork().Ingress.Public.Allow, []string{}, func(acc []string, addr radixv1.IPOrCIDR) []string {
return append(acc, string(addr))
})
return map[string]string{"nginx.ingress.kubernetes.io/whitelist-source-range": strings.Join(addressList, ",")}, nil

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, but used slice.Map instead of Reduce

}
50 changes: 50 additions & 0 deletions pkg/apis/ingress/ingressannotationprovider_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,56 @@ func Test_ClientCertificateAnnotations(t *testing.T) {
assert.Empty(t, result, "Expected Annotations to be empty")
}

func Test_PublicIngressAllowAnnotationProvider(t *testing.T) {
tests := map[string]struct {
component radixv1.RadixCommonDeployComponent
expectedAnnotation map[string]string
}{
"network is not set": {
component: &radixv1.RadixDeployComponent{},
expectedAnnotation: make(map[string]string),
},
"network.ingress is not set": {
component: &radixv1.RadixDeployComponent{Network: &radixv1.Network{}},
expectedAnnotation: make(map[string]string),
},
"network.ingress.public is not set": {
component: &radixv1.RadixDeployComponent{Network: &radixv1.Network{Ingress: &radixv1.Ingress{}}},
expectedAnnotation: make(map[string]string),
},
"network.ingress.public.allow is not set": {
component: &radixv1.RadixDeployComponent{Network: &radixv1.Network{Ingress: &radixv1.Ingress{Public: &radixv1.IngressPublic{}}}},
expectedAnnotation: make(map[string]string),
},
"network.ingress.public.allow is empty": {
component: &radixv1.RadixDeployComponent{Network: &radixv1.Network{Ingress: &radixv1.Ingress{Public: &radixv1.IngressPublic{Allow: &[]radixv1.IPOrCIDR{}}}}},
expectedAnnotation: make(map[string]string),
},
"network.ingress.public.allow single entry": {
component: &radixv1.RadixDeployComponent{Network: &radixv1.Network{Ingress: &radixv1.Ingress{Public: &radixv1.IngressPublic{Allow: &[]radixv1.IPOrCIDR{radixv1.IPOrCIDR("10.0.0.1")}}}}},
expectedAnnotation: map[string]string{"nginx.ingress.kubernetes.io/whitelist-source-range": "10.0.0.1"},
},
"network.ingress.public.allow multiple entries": {
component: &radixv1.RadixDeployComponent{Network: &radixv1.Network{Ingress: &radixv1.Ingress{Public: &radixv1.IngressPublic{Allow: &[]radixv1.IPOrCIDR{radixv1.IPOrCIDR("10.0.0.1"), radixv1.IPOrCIDR("10.10.10.10/30")}}}}},
expectedAnnotation: map[string]string{"nginx.ingress.kubernetes.io/whitelist-source-range": "10.0.0.1,10.10.10.10/30"},
},
}

sut := &ingressPublicAllowListAnnotationProvider{}
for testName, testSpec := range tests {
t.Run(testName, func(t *testing.T) {
actual, err := sut.GetAnnotations(testSpec.component, "")
assert.NoError(t, err)
if len(testSpec.expectedAnnotation) == 0 {
assert.Empty(t, actual)
} else {
assert.Equal(t, actual, testSpec.expectedAnnotation)
}
})
}

}

type OAuth2AnnotationsTestSuite struct {
suite.Suite
oauth2Config *defaults.MockOAuth2Config
Expand Down
5 changes: 4 additions & 1 deletion pkg/apis/ingress/oauth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@ import (

// GetAuxOAuthProxyAnnotationProviders Gets aux OAuth proxy annotation providers
func GetAuxOAuthProxyAnnotationProviders() []AnnotationProvider {
return []AnnotationProvider{NewForceSslRedirectAnnotationProvider()}
return []AnnotationProvider{
NewForceSslRedirectAnnotationProvider(),
NewIngressPublicAllowListAnnotationProvider(),
}
}

// BuildOAuthProxyIngressForComponentIngress builds OAuth proxy ingress for RadixDeploy component ingress
Expand Down
Loading
Loading