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

create clusterwidenetworkpolicy from AccessList.SourceRanges #67

Merged
merged 11 commits into from
Feb 8, 2021
Merged
3 changes: 3 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -154,3 +154,6 @@ helm:
helm package charts/postgreslet-support/
helm dependency build charts/postgreslet/
helm package charts/postgreslet/

test-cwnp:
./hack/test-cwnp.sh
49 changes: 47 additions & 2 deletions api/v1/postgres_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,17 @@ limitations under the License.
package v1

import (
"errors"
"fmt"
"time"

firewall "github.com/metal-stack/firewall-controller/api/v1"
"inet.af/netaddr"
corev1 "k8s.io/api/core/v1"
networkingv1 "k8s.io/api/networking/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/intstr"
)

// EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN!
Expand Down Expand Up @@ -80,8 +87,7 @@ type PostgresSpec struct {

// AccessList defines the type of restrictions to access the database
type AccessList struct {
// SourceRanges defines a list of prefixes in CIDR Notation e.g. 1.2.3.0/24
// FIXME implement validation if source is a parsable CIDR
// SourceRanges defines a list of prefixes in CIDR Notation e.g. 1.2.3.0/24 or fdaa::/104
SourceRanges []string `json:"sourceRanges,omitempty"`
}

Expand Down Expand Up @@ -152,11 +158,50 @@ type PostgresList struct {
Items []Postgres `json:"items"`
}

// HasSourceRanges returns true if SourceRanges are set
func (p *Postgres) HasSourceRanges() bool {
return p.Spec.AccessList.SourceRanges != nil
}

// IsBeingDeleted returns true if the deletion-timestamp is set
func (p *Postgres) IsBeingDeleted() bool {
return !p.ObjectMeta.DeletionTimestamp.IsZero()
}

// ToCWNP returns CRD ClusterwideNetworkPolic derived from CRD Postgres
Copy link
Collaborator

Choose a reason for hiding this comment

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

The ClusterwideNetworkPolic is missing a y :-)

func (p *Postgres) ToCWNP(port int) (*firewall.ClusterwideNetworkPolicy, error) {
if !p.HasSourceRanges() {
return nil, errors.New(".spec.accessList.sourceRanges not set")
eberlep marked this conversation as resolved.
Show resolved Hide resolved
}

portObj := intstr.FromInt(port)
tcp := corev1.ProtocolTCP
ports := []networkingv1.NetworkPolicyPort{
{Port: &portObj, Protocol: &tcp},
}

ipblocks := []networkingv1.IPBlock{}
for _, src := range p.Spec.AccessList.SourceRanges {
parsedSrc, err := netaddr.ParseIPPrefix(src)
if err != nil {
return nil, fmt.Errorf("unable to parse source range %s: %w", src, err)
}
ipblock := networkingv1.IPBlock{
CIDR: parsedSrc.String(),
}
ipblocks = append(ipblocks, ipblock)
}

policy := &firewall.ClusterwideNetworkPolicy{}
policy.Namespace = "firewall"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a const somewhere in the firewall package that we can use instead of this hardcoded string? If this changes in the control plane cluster / firewall controller, this cwnp might not be loaded.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

policy.Name = p.Spec.ProjectID + "--" + string(p.UID)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should use the same, centralized logic for generating a name as the postgresql resources so that both names match.

Currently, this is the case (p.Spec.ProjectID + "--" + string(p.UID)), but we have the same implementation in different places, so if one of them changes, those names will differ.

policy.Spec.Ingress = []firewall.IngressRule{
{Ports: ports, From: ipblocks},
}

return policy, nil
}

func (p *Postgres) ToKey() *types.NamespacedName {
return &types.NamespacedName{
Namespace: p.Namespace,
Expand Down
3 changes: 1 addition & 2 deletions config/crd/bases/database.fits.cloud_postgres.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,7 @@ spec:
properties:
sourceRanges:
description: SourceRanges defines a list of prefixes in CIDR Notation
e.g. 1.2.3.0/24 FIXME implement validation if source is a parsable
CIDR
e.g. 1.2.3.0/24 or fdaa::/104
items:
type: string
type: array
Expand Down
23 changes: 23 additions & 0 deletions config/samples/_test_cwnp.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
apiVersion: v1
kind: Namespace
metadata:
name: database
---
apiVersion: database.fits.cloud/v1
kind: Postgres
metadata:
namespace: database
name: sample-name-a
spec:
accessList:
sourceRanges:
- 1.2.3.4/24
backup:
s3BucketURL: ""
numberOfInstances: 2
partitionID: sample-partition
projectID: projectid-a
size:
storageSize: 1Gi
tenant: sample-tenant
version: "12"
6 changes: 6 additions & 0 deletions config/samples/database_v1_postgres.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ metadata:
namespace: database
name: sample-name-a
spec:
accessList:
sourceRanges:
- 1.2.3.4/24
backup:
s3BucketURL: ""
numberOfInstances: 2
Expand All @@ -25,6 +28,9 @@ metadata:
namespace: database
name: sample-name-b
spec:
accessList:
sourceRanges:
- 1.2.3.4/24
backup:
s3BucketURL: ""
numberOfInstances: 2
Expand Down
49 changes: 48 additions & 1 deletion controllers/postgres_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,15 @@ import (
"github.com/go-logr/logr"
zalando "github.com/zalando/postgres-operator/pkg/apis/acid.zalan.do/v1"
corev1 "k8s.io/api/core/v1"

firewall "github.com/metal-stack/firewall-controller/api/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"

pg "github.com/fi-ts/postgres-controller/api/v1"
"github.com/fi-ts/postgres-controller/pkg/operatormanager"
Expand Down Expand Up @@ -76,6 +80,13 @@ func (r *PostgresReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {

// Delete
if instance.IsBeingDeleted() {
if instance.HasSourceRanges() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe always delete, but ignore NotFound errors? This would make sure we always delete it, even if the postgres resource was modified in between (e.g. the sourceRange was removed).

Copy link
Contributor

Choose a reason for hiding this comment

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

We will always delete the corresponding ClusterwideNetworkPolicy. Since we've decided there will always be one such ClusterwideNetworkPolicy for every Postgres, the NotFound error will not be ignored.

if err := r.deleteCWNP(ctx, instance); err != nil {
return ctrl.Result{}, err
}
log.Info("corresponding CRD ClusterwideNetworkPolicy deleted")
}

log.Info("deleting owned zalando postgresql")
if err := r.deleteZPostgresql(ctx, k); err != nil {
return ctrl.Result{}, err
Expand Down Expand Up @@ -142,7 +153,11 @@ func (r *PostgresReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
log.Info("zalando postgresql updated", "ns/name", k)
}

// todo: what port?
// Update status will be handled by the StatusReconciler, based on the Zalando Status
if err := r.createOrUpdateCWNP(ctx, instance, 12345); err != nil {
return ctrl.Result{}, fmt.Errorf("unable to create or update corresponding CRD ClusterwideNetworkPolicy: %W", err)
}

return ctrl.Result{}, nil
}
Expand Down Expand Up @@ -236,6 +251,7 @@ func (r *PostgresReconciler) deleteZPostgresql(ctx context.Context, k *types.Nam
}

func patchRawZ(out *zalando.Postgresql, in *pg.Postgres) {
out.Spec.AllowedSourceRanges = in.Spec.AccessList.SourceRanges
out.Spec.NumberOfInstances = in.Spec.NumberOfInstances

// todo: Check if the validation should be performed here.
Expand Down Expand Up @@ -266,5 +282,36 @@ func patchRawZ(out *zalando.Postgresql, in *pg.Postgres) {
}
}()

// todo: in.Spec.Backup, in.Spec.AccessList
// todo: in.Spec.Backup
}

// todo: Change to `controllerutl.CreateOrPatch`
// createOrUpdateCWNP will create an ingress firewall rule on the firewall in front of the k8s cluster
// based on the spec.AccessList.SourceRanges given.
eberlep marked this conversation as resolved.
Show resolved Hide resolved
func (r *PostgresReconciler) createOrUpdateCWNP(ctx context.Context, in *pg.Postgres, port int) error {
policy, err := in.ToCWNP(port)
if err != nil {
return fmt.Errorf("unable to convert instance to CRD ClusterwideNetworkPolicy: %w", err)
}

// placeholder of the object with the specified namespaced name
key := &firewall.ClusterwideNetworkPolicy{ObjectMeta: metav1.ObjectMeta{Name: policy.Name, Namespace: policy.Namespace}}
if _, err := controllerutil.CreateOrUpdate(ctx, r.Service, key, func() error {
key.Spec.Ingress = policy.Spec.Ingress
return nil
}); err != nil {
return fmt.Errorf("unable to deploy CRD ClusterwideNetworkPolicy: %w", err)
}

return nil
}

func (r *PostgresReconciler) deleteCWNP(ctx context.Context, in *pg.Postgres) error {
policy := &firewall.ClusterwideNetworkPolicy{}
policy.Namespace = "firewall"
eberlep marked this conversation as resolved.
Show resolved Hide resolved
policy.Name = in.Spec.ProjectID + "--" + string(in.UID)
eberlep marked this conversation as resolved.
Show resolved Hide resolved
if err := r.Service.Delete(ctx, policy); err != nil {
return fmt.Errorf("unable to delete CRD ClusterwideNetworkPolicy %v: %w", policy.Name, err)
}
return nil
}
5 changes: 3 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@ go 1.15

require (
github.com/go-logr/logr v0.3.0
github.com/go-logr/zapr v0.3.0 // indirect
github.com/metal-stack/firewall-controller v1.0.1
github.com/metal-stack/v v1.0.2
github.com/onsi/ginkgo v1.14.2
github.com/onsi/gomega v1.10.4
github.com/zalando/postgres-operator v1.5.0
inet.af/netaddr v0.0.0-20210115183222-bffc12a571f6
k8s.io/api v0.19.4
k8s.io/apiextensions-apiserver v0.18.6
k8s.io/apiextensions-apiserver v0.18.9
k8s.io/apimachinery v0.19.4
k8s.io/client-go v11.0.0+incompatible
sigs.k8s.io/controller-runtime v0.6.4
Expand Down
Loading