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.

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

Implement orchestrated firewall distance configuration. #24

Merged
merged 17 commits into from
Apr 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
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