Skip to content

Commit

Permalink
Implement orchestrated firewall distance configuration. (#24)
Browse files Browse the repository at this point in the history
  • Loading branch information
Gerrit91 authored Apr 13, 2023
1 parent fa2d09f commit 7303a6c
Show file tree
Hide file tree
Showing 23 changed files with 392 additions and 124 deletions.
63 changes: 42 additions & 21 deletions api/v2/types_firewall.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ const (
// +kubebuilder:printcolumn:name="Phase",type="string",JSONPath=".status.phase"
// +kubebuilder:printcolumn:name="Machine ID",type="string",JSONPath=".status.machineStatus.machineID"
// +kubebuilder:printcolumn:name="Last Event",type="string",JSONPath=".status.machineStatus.lastEvent.event"
// +kubebuilder:printcolumn:name="Distance",type="string",priority=1,JSONPath=".distance"
// +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".status.machineStatus.allocationTimestamp"
// +kubebuilder:printcolumn:name="Spec Version",type="string",priority=1,JSONPath=".spec.controllerVersion"
// +kubebuilder:printcolumn:name="Actual Version",type="string",priority=1,JSONPath=".status.controllerStatus.actualVersion"
Expand All @@ -47,6 +48,10 @@ type Firewall struct {
Spec FirewallSpec `json:"spec"`
// Status contains current status information on the firewall.
Status FirewallStatus `json:"status,omitempty"`

// Distance defines the as-path length of a firewall.
// This field is typically orchestrated by the deployment controller.
Distance FirewallDistance `json:"distance"`
}

// FirewallSpec defines parameters for the firewall creation along with configuration for the firewall-controller.
Expand Down Expand Up @@ -100,6 +105,7 @@ type FirewallSpec struct {

// LogAcceptedConnections if set to true, also log accepted connections in the droptailer log.
LogAcceptedConnections bool `json:"logAcceptedConnections,omitempty"`

// DNSServerAddress specifies DNS server address used by DNS proxy
DNSServerAddress string `json:"dnsServerAddress,omitempty"`
// DNSPort specifies port to which DNS proxy should be bound
Expand Down Expand Up @@ -172,6 +178,8 @@ const (
FirewallControllerConnected ConditionType = "Connected"
// FirewallMonitorDeployed indicates that the firewall monitor is deployed into the shoot cluster
FirewallMonitorDeployed ConditionType = "MonitorDeployed"
// FirewallDistanceConfigured indicates that the firewall-controller has configured the given firewall distance.
FirewallDistanceConfigured ConditionType = "Distance"
)

// ShootAccess contains secret references to construct a shoot client in the firewall-controller to update its firewall monitor.
Expand Down Expand Up @@ -222,6 +230,8 @@ type ControllerConnection struct {
ActualVersion string `json:"actualVersion,omitempty"`
// Updated is a timestamp when the controller has last reconciled the firewall resource.
Updated metav1.Time `json:"lastRun,omitempty"`
// ActualDistance is the actual distance as reflected by the firewall-controller.
ActualDistance FirewallDistance `json:"actualDistance,omitempty"`
}

// FirewallNetwork holds refined information about a network that the firewall is connected to.
Expand Down Expand Up @@ -264,37 +274,39 @@ func (f *FirewallList) GetItems() []*Firewall {
return result
}

// SortFirewallsDeletion sorts the given firewall slice for deletion.
// SortFirewallsByImportance sorts the given firewall slice by importance,
// e.g. for scale down.
//
// It considers certain criteria which firewalls should be kept longest and
// which one's can be deleted first. The precedence is:
//
// - Weight annotation (defaults to 0 if no annotation is present)
// - Connected firewalls
// - Ready firewalls
// - Created Firealls
// - Younger Firewalls
// - Weight annotation (prefer higher weight, defaults to 0 if no annotation is present)
// - Firewall lifecycle phase (connected > ready > created, prefer shorter distance when equal)
// - Firewall age (prefer younger firewalls)
//
// The firewalls at the beginning of the slice should be kept as long as possible.
// The firewalls at the end of the slice should be removed first.
//
// The firewalls can be popped off from the slice in a deletion loop.
func SortFirewallsDeletion(fws []*Firewall) {
weight := func(fw *Firewall) (weight int) {
a, ok := fw.Annotations[FirewallWeightAnnotation]
if !ok {
return
}
func SortFirewallsByImportance(fws []*Firewall) {
var (
conditionTypes = []ConditionType{FirewallControllerConnected, FirewallReady, FirewallCreated}

weight = func(fw *Firewall) (weight int) {
a, ok := fw.Annotations[FirewallWeightAnnotation]
if !ok {
return
}

parsed, err := strconv.ParseInt(a, 10, 32)
if err != nil {
parsed, err := strconv.ParseInt(a, 10, 32)
if err != nil {
return
}

weight = int(parsed)
return
}

weight = int(parsed)
return
}

conditionTypes := []ConditionType{FirewallControllerConnected, FirewallReady, FirewallCreated}
)

sort.Slice(fws, func(i, j int) bool {
a := fws[i]
Expand All @@ -319,9 +331,18 @@ func SortFirewallsDeletion(fws []*Firewall) {
if !aTrue && bTrue {
return false
}
if aTrue && bTrue {
// prefer shorter distances because these are potentially "active"
if a.Distance < b.Distance {
return true
}
if a.Distance > b.Distance {
return false
}
}
}

// prefer younger firewalls
// prefer younger firewalls (these potentially run on a more up-to-date operating system image)
return !a.CreationTimestamp.Before(&b.CreationTimestamp)
})
}
18 changes: 12 additions & 6 deletions api/v2/types_firewall_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

func TestSortForScaleDown(t *testing.T) {
func Test_SortFirewallsByImportance(t *testing.T) {
now := time.Now()

tests := []struct {
Expand All @@ -32,14 +32,17 @@ func TestSortForScaleDown(t *testing.T) {
{
ObjectMeta: metav1.ObjectMeta{Name: "lowest weight", Annotations: map[string]string{FirewallWeightAnnotation: "-100"}},
},
{
ObjectMeta: metav1.ObjectMeta{Name: "connected shortest distance", CreationTimestamp: metav1.NewTime(now.Add(-20 * time.Minute))}, Distance: FirewallShortestDistance, Status: FirewallStatus{Conditions: Conditions{NewCondition(FirewallControllerConnected, ConditionTrue, "Connected", ""), NewCondition(FirewallReady, ConditionTrue, "Ready", "")}},
},
{
ObjectMeta: metav1.ObjectMeta{Name: "middle", CreationTimestamp: metav1.NewTime(now)},
},
{
ObjectMeta: metav1.ObjectMeta{Name: "created"}, Status: FirewallStatus{Conditions: Conditions{NewCondition(FirewallCreated, ConditionTrue, "Created", "")}},
},
{
ObjectMeta: metav1.ObjectMeta{Name: "connected younger", CreationTimestamp: metav1.NewTime(now.Add(5 * time.Minute))}, Status: FirewallStatus{Conditions: Conditions{NewCondition(FirewallControllerConnected, ConditionTrue, "Connected", ""), NewCondition(FirewallReady, ConditionTrue, "Ready", "")}},
ObjectMeta: metav1.ObjectMeta{Name: "connected younger", CreationTimestamp: metav1.NewTime(now.Add(5 * time.Minute))}, Distance: FirewallShortestDistance + 1, Status: FirewallStatus{Conditions: Conditions{NewCondition(FirewallControllerConnected, ConditionTrue, "Connected", ""), NewCondition(FirewallReady, ConditionTrue, "Ready", "")}},
},
{
ObjectMeta: metav1.ObjectMeta{Name: "ready younger", CreationTimestamp: metav1.NewTime(now.Add(5 * time.Minute))}, Status: FirewallStatus{Conditions: Conditions{NewCondition(FirewallReady, ConditionTrue, "Ready", "")}},
Expand All @@ -51,18 +54,21 @@ func TestSortForScaleDown(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{Name: "no information"},
},
{
ObjectMeta: metav1.ObjectMeta{Name: "connected older", CreationTimestamp: metav1.NewTime(now.Add(-5 * time.Minute))}, Status: FirewallStatus{Conditions: Conditions{NewCondition(FirewallControllerConnected, ConditionTrue, "Connected", ""), NewCondition(FirewallReady, ConditionTrue, "Ready", "")}},
ObjectMeta: metav1.ObjectMeta{Name: "connected older", CreationTimestamp: metav1.NewTime(now.Add(-5 * time.Minute))}, Distance: FirewallShortestDistance + 1, Status: FirewallStatus{Conditions: Conditions{NewCondition(FirewallControllerConnected, ConditionTrue, "Connected", ""), NewCondition(FirewallReady, ConditionTrue, "Ready", "")}},
},
},
want: []*Firewall{
{
ObjectMeta: metav1.ObjectMeta{Name: "highest weight", Annotations: map[string]string{FirewallWeightAnnotation: "100"}},
},
{
ObjectMeta: metav1.ObjectMeta{Name: "connected younger", CreationTimestamp: metav1.NewTime(now.Add(5 * time.Minute))}, Status: FirewallStatus{Conditions: Conditions{NewCondition(FirewallControllerConnected, ConditionTrue, "Connected", ""), NewCondition(FirewallReady, ConditionTrue, "Ready", "")}},
ObjectMeta: metav1.ObjectMeta{Name: "connected shortest distance", CreationTimestamp: metav1.NewTime(now.Add(-20 * time.Minute))}, Distance: FirewallShortestDistance, Status: FirewallStatus{Conditions: Conditions{NewCondition(FirewallControllerConnected, ConditionTrue, "Connected", ""), NewCondition(FirewallReady, ConditionTrue, "Ready", "")}},
},
{
ObjectMeta: metav1.ObjectMeta{Name: "connected younger", CreationTimestamp: metav1.NewTime(now.Add(5 * time.Minute))}, Distance: FirewallShortestDistance + 1, Status: FirewallStatus{Conditions: Conditions{NewCondition(FirewallControllerConnected, ConditionTrue, "Connected", ""), NewCondition(FirewallReady, ConditionTrue, "Ready", "")}},
},
{
ObjectMeta: metav1.ObjectMeta{Name: "connected older", CreationTimestamp: metav1.NewTime(now.Add(-5 * time.Minute))}, Status: FirewallStatus{Conditions: Conditions{NewCondition(FirewallControllerConnected, ConditionTrue, "Connected", ""), NewCondition(FirewallReady, ConditionTrue, "Ready", "")}},
ObjectMeta: metav1.ObjectMeta{Name: "connected older", CreationTimestamp: metav1.NewTime(now.Add(-5 * time.Minute))}, Distance: FirewallShortestDistance + 1, Status: FirewallStatus{Conditions: Conditions{NewCondition(FirewallControllerConnected, ConditionTrue, "Connected", ""), NewCondition(FirewallReady, ConditionTrue, "Ready", "")}},
},
{
ObjectMeta: metav1.ObjectMeta{Name: "ready younger", CreationTimestamp: metav1.NewTime(now.Add(5 * time.Minute))}, Status: FirewallStatus{Conditions: Conditions{NewCondition(FirewallReady, ConditionTrue, "Ready", "")}},
Expand Down Expand Up @@ -94,7 +100,7 @@ func TestSortForScaleDown(t *testing.T) {
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
SortFirewallsDeletion(tt.fws)
SortFirewallsByImportance(tt.fws)

if diff := cmp.Diff(tt.want, tt.fws, cmpopts.IgnoreFields(Condition{}, "LastTransitionTime", "LastUpdateTime")); diff != "" {
t.Errorf("diff (+got -want):\n %s", diff)
Expand Down
1 change: 1 addition & 0 deletions api/v2/types_firewalldeployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
// +kubebuilder:object:root=true
// +kubebuilder:resource:shortName=fwdeploy
// +kubebuilder:subresource:status
// +kubebuilder:subresource:scale:specpath=.spec.replicas,statuspath=.status.readyReplicas
// +kubebuilder:printcolumn:name="Replicas",type=integer,JSONPath=`.spec.replicas`
// +kubebuilder:printcolumn:name="Ready",type=integer,JSONPath=`.status.readyReplicas`
// +kubebuilder:printcolumn:name="Progressing",type=integer,JSONPath=`.status.progressingReplicas`
Expand Down
12 changes: 7 additions & 5 deletions api/v2/types_firewallmonitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,13 @@ type FirewallMonitor struct {
}

type ControllerStatus struct {
Message string `json:"message,omitempty"`
FirewallStats *FirewallStats `json:"stats,omitempty"`
ControllerVersion string `json:"controllerVersion,omitempty"`
NftablesExporterVersion string `json:"nftablesExporterVersion,omitempty"`
Updated metav1.Time `json:"lastRun,omitempty"`
Message string `json:"message,omitempty"`
FirewallStats *FirewallStats `json:"stats,omitempty"`
ControllerVersion string `json:"controllerVersion,omitempty"`
NftablesExporterVersion string `json:"nftablesExporterVersion,omitempty"`
Updated metav1.Time `json:"lastRun,omitempty"`
Distance FirewallDistance `json:"distance,omitempty"`
DistanceSupported bool `json:"distanceSupported,omitempty"`
}

// FirewallStats contains firewall statistics
Expand Down
26 changes: 26 additions & 0 deletions api/v2/types_firewallset.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,14 @@ import (
const (
// FirewallControllerSetAnnotation is a tag added to the firewall entity indicating to which set a firewall belongs to.
FirewallControllerSetAnnotation = "firewall.metal.stack.io/set"

FirewallShortestDistance = FirewallDistance(0)
FirewallRollingUpdateSetDistance = FirewallDistance(3)
FirewallLongestDistance = FirewallDistance(8)

// FirewallMaxReplicas defines the maximum amount of firewall replicas to be defined.
// It does not make sense to allow large values here as it wastes a lot of machines.
FirewallMaxReplicas = 4
)

func FirewallSetTag(setName string) string {
Expand All @@ -27,10 +35,12 @@ func FirewallManagedByTag() string {
// +kubebuilder:object:root=true
// +kubebuilder:resource:shortName=fwset
// +kubebuilder:subresource:status
// +kubebuilder:subresource:scale:specpath=.spec.replicas,statuspath=.status.readyReplicas
// +kubebuilder:printcolumn:name="Replicas",type=integer,JSONPath=`.spec.replicas`
// +kubebuilder:printcolumn:name="Ready",type=integer,JSONPath=`.status.readyReplicas`
// +kubebuilder:printcolumn:name="Progressing",type=integer,JSONPath=`.status.progressingReplicas`
// +kubebuilder:printcolumn:name="Unhealthy",type=integer,JSONPath=`.status.unhealthyReplicas`
// +kubebuilder:printcolumn:name="Distance",type="string",priority=1,JSONPath=".spec.distance"
// +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp"
type FirewallSet struct {
metav1.TypeMeta `json:",inline"`
Expand All @@ -53,8 +63,24 @@ type FirewallSetSpec struct {
Selector map[string]string `json:"selector,omitempty"`
// Template is the firewall spec used for creating the firewalls.
Template FirewallTemplateSpec `json:"template"`
// Distance defines the as-path length of the firewalls.
// This field is typically orchestrated by the deployment controller.
Distance FirewallDistance `json:"distance"`
}

// FirewallDistance defines the as-path length of firewalls, influencing how strong they attract
// network traffic for routing traffic in and out of the cluster.
// This is of particular interest during rolling firewall updates, i.e. when there is
// more than a single firewall running in front of the cluster.
// During a rolling update, new firewalls start with a longer distance such that
// traffic is only attracted by the existing firewalls ("firewall staging").
// When the new firewall has connected successfully to the firewall monitor, the deployment
// controller throws away the old firewalls and the new firewall takes over the routing.
// The deployment controller will then shorten the distance of the new firewall.
// This approach reduces service interruption of the external user traffic of the cluster
// (for firewall-controller versions that support this feature).
type FirewallDistance uint8

type FirewallSetStatus struct {
// TargetReplicas is the amount of firewall replicas targeted to be running.
TargetReplicas int `json:"targetReplicas"`
Expand Down
12 changes: 12 additions & 0 deletions api/v2/validation/firewall.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ func (v *firewallValidator) ValidateCreate(log logr.Logger, f *v2.Firewall) fiel

allErrs = append(allErrs, validateFirewallAnnotations(f)...)
allErrs = append(allErrs, v.validateSpec(&f.Spec, field.NewPath("spec"))...)
allErrs = append(allErrs, validateDistance(f.Distance, field.NewPath("distance"))...)

return allErrs
}
Expand Down Expand Up @@ -102,6 +103,7 @@ func (v *firewallValidator) ValidateUpdate(log logr.Logger, fOld, fNew *v2.Firew

allErrs = append(allErrs, validateFirewallAnnotations(fNew)...)
allErrs = append(allErrs, v.validateSpecUpdate(&fOld.Spec, &fNew.Spec, field.NewPath("spec"))...)
allErrs = append(allErrs, validateDistance(fNew.Distance, field.NewPath("distance"))...)

return allErrs
}
Expand Down Expand Up @@ -136,3 +138,13 @@ func validateFirewallAnnotations(f *v2.Firewall) field.ErrorList {

return allErrs
}

func validateDistance(distance v2.FirewallDistance, fldPath *field.Path) field.ErrorList {
var allErrs field.ErrorList

if distance < v2.FirewallShortestDistance || distance > v2.FirewallLongestDistance {
allErrs = append(allErrs, field.Invalid(fldPath, distance, fmt.Sprintf("distance must be between %d and %d", v2.FirewallShortestDistance, v2.FirewallLongestDistance)))
}

return allErrs
}
3 changes: 3 additions & 0 deletions api/v2/validation/firewall_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ func Test_firewallValidator_ValidateCreate(t *testing.T) {
},
},
},
Distance: 0,
}

tests := []struct {
Expand Down Expand Up @@ -141,6 +142,7 @@ func Test_firewallValidator_ValidateUpdate(t *testing.T) {
},
},
},
Distance: 0,
},
oldF: &v2.Firewall{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -171,6 +173,7 @@ func Test_firewallValidator_ValidateUpdate(t *testing.T) {
},
},
},
Distance: 0,
},
wantErr: nil,
},
Expand Down
4 changes: 2 additions & 2 deletions api/v2/validation/firewalldeployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ func (*firewallDeploymentValidator) validateSpec(log logr.Logger, f *v2.Firewall
if f.Replicas < 0 {
allErrs = append(allErrs, field.Invalid(fldPath.Child("replicas"), f.Replicas, "replicas cannot be a negative number"))
}
if f.Replicas > 1 {
allErrs = append(allErrs, field.Invalid(fldPath.Child("replicas"), f.Replicas, "for now, no more than a single firewall replica is allowed"))
if f.Replicas > v2.FirewallMaxReplicas {
allErrs = append(allErrs, field.Invalid(fldPath.Child("replicas"), f.Replicas, fmt.Sprintf("no more than %d firewall replicas are allowed", v2.FirewallMaxReplicas)))
}

if f.Selector == nil {
Expand Down
7 changes: 7 additions & 0 deletions api/v2/validation/firewallset.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package validation

import (
"fmt"

"github.com/go-logr/logr"
v2 "github.com/metal-stack/firewall-controller-manager/api/v2"
apivalidation "k8s.io/apimachinery/pkg/api/validation"
Expand Down Expand Up @@ -29,6 +31,11 @@ func (v *firewallSetValidator) validateSpec(log logr.Logger, f *v2.FirewallSetSp
if f.Replicas < 0 {
allErrs = append(allErrs, field.Invalid(fldPath.Child("replicas"), f.Replicas, "replicas cannot be a negative number"))
}
if f.Replicas > v2.FirewallMaxReplicas {
allErrs = append(allErrs, field.Invalid(fldPath.Child("replicas"), f.Replicas, fmt.Sprintf("no more than %d firewall replicas are allowed", v2.FirewallMaxReplicas)))
}

allErrs = append(allErrs, validateDistance(f.Distance, fldPath.Child("distance"))...)

if f.Selector == nil {
allErrs = append(allErrs, field.Invalid(fldPath.Child("selector"), f.Selector, "selector should not be nil"))
Expand Down
14 changes: 14 additions & 0 deletions api/v2/validation/firewallset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ func Test_firewalSetValidator_ValidateCreate(t *testing.T) {
Selector: map[string]string{
"purpose": "shoot-firewall",
},
Distance: 0,
Template: v2.FirewallTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
Expand Down Expand Up @@ -109,6 +110,18 @@ func Test_firewalSetValidator_ValidateCreate(t *testing.T) {
},
},
},
{
name: "no more than max allowed replicas",
mutateFn: func(f *v2.FirewallSet) *v2.FirewallSet {
f.Spec.Replicas = 200
return f
},
wantErr: &apierrors.StatusError{
ErrStatus: metav1.Status{
Message: ` "firewall" is invalid: spec.replicas: Invalid value: 200: no more than 4 firewall replicas are allowed`,
},
},
},
}
for _, tt := range tests {
tt := tt
Expand All @@ -134,6 +147,7 @@ func Test_firewallSetValidator_ValidateUpdate(t *testing.T) {
"purpose": "shoot-firewall",
"a": "b",
},
Distance: 0,
Template: v2.FirewallTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
Expand Down
3 changes: 3 additions & 0 deletions config/crds/firewall.metal-stack.io_firewalldeployments.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -316,4 +316,7 @@ spec:
served: true
storage: true
subresources:
scale:
specReplicasPath: .spec.replicas
statusReplicasPath: .status.readyReplicas
status: {}
Loading

0 comments on commit 7303a6c

Please sign in to comment.