create clusterwidenetworkpolicy from AccessList.SourceRanges #67
Conversation
api/v1/postgres_types.go
Outdated
| @@ -75,6 +75,7 @@ type PostgresSpec struct { | |||
|
|
|||
| // AccessList defines the type of restrictions to access the database | |||
| type AccessList struct { | |||
| // +kubebuilder:validation:Required | |||
There was a problem hiding this comment.
SourceRanges might be empty list if customer doesn't care
LimKianAn
changed the title
api/v1/postgres_types.go
Outdated
| // 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 |
There was a problem hiding this comment.
The ClusterwideNetworkPolic is missing a y :-)
api/v1/postgres_types.go
Outdated
|
|
||
| policy := &firewall.ClusterwideNetworkPolicy{} | ||
| policy.Namespace = "firewall" | ||
| policy.Name = p.Spec.ProjectID + "--" + string(p.UID) |
There was a problem hiding this comment.
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 := &firewall.ClusterwideNetworkPolicy{} | ||
| policy.Namespace = "firewall" |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
no there is no exported const: https://github.com/metal-stack/firewall-controller/blob/master/controllers/clusterwidenetworkpolicy_controller.go#L40
controllers/postgres_controller.go
Outdated
| @@ -76,6 +80,13 @@ func (r *PostgresReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { | |||
|
|
|||
| // Delete | |||
| if instance.IsBeingDeleted() { | |||
| if instance.HasSourceRanges() { | |||
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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.
Denying all access is default
|
Firewall Controller PR: metal-stack/firewall-controller#71 |
This is just to show how this could be done, please integrate into your controller logic as you like.
Obviously is not called, therefore the build/lint is failing !