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

[Feat] Enable adding NodeBalancer to Linode Firewall #539

Merged
merged 31 commits into from
Oct 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
b247f0e
feat: initial implementation to enable adding nb to firewall
Oct 10, 2024
f493713
oops forgot to run make generate :)
Oct 10, 2024
d69b182
feat: adding retry functionality if there is a credential finalizer u…
Oct 14, 2024
6421713
feat: Create a new NB firewall and pass it through FirewallRef in on …
Oct 14, 2024
ef5a6e4
feat: Add the ability to allow adding and configuring Firewall alread…
Oct 14, 2024
3deaca0
Isolatie the retry logic for finalizer update to just firewall and no…
Oct 15, 2024
0a8d98c
Merge branch 'main' into nb-firewall
Oct 15, 2024
f8a0a82
fix: Change the firewallref to nodeBalancerFirewallRef & nodeBalancer…
Oct 15, 2024
ab1b01e
oops forgot to remove this comment
Oct 15, 2024
daa2d0f
Adding preflight check for firewall creation and availablity
Oct 16, 2024
3c089ac
fixup: lint err
Oct 16, 2024
a7f06c8
Switching to a new conditiontype specific for NB firewall and fixing …
Oct 16, 2024
e3e14dd
fixup: address the test case that was failing
Oct 16, 2024
85a334e
Merge branch 'main' into nb-firewall
Oct 16, 2024
4061f5e
revert
Oct 16, 2024
3ab3272
lets try refactor again
Oct 16, 2024
ec922f9
Updating and adding e2e test for nb firewall
Oct 16, 2024
fdf595e
move the ConditionPreflightLinodeNBFirewallReady to linodecluster file
Oct 16, 2024
b7711f2
Merge branch 'main' into nb-firewall
Oct 16, 2024
105509f
fix: nb firewall was not coming up for some e2e test because of char …
Oct 17, 2024
94c5e11
various template updates
Oct 17, 2024
57c1881
fix: update the kustomize template to include konnectivity ports
Oct 17, 2024
308981f
yamllint fix
Oct 17, 2024
7e433ba
Add quotes to konnectivity port in nb
Oct 17, 2024
d091c8c
syntax fix for firewall
Oct 17, 2024
ebab2bc
Merge branch 'main' into nb-firewall
Oct 22, 2024
637abcc
Update/Add test cases
Oct 23, 2024
c48ba4a
lint fix
Oct 23, 2024
61f69c0
Merge branch 'main' into nb-firewall
Oct 23, 2024
03216e4
lint fix
Oct 23, 2024
f5c18bd
lint fix
Oct 23, 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
2 changes: 2 additions & 0 deletions api/v1alpha1/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions api/v1alpha2/linodecluster_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ type LinodeClusterSpec struct {
// +optional
VPCRef *corev1.ObjectReference `json:"vpcRef,omitempty"`

// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
komer3 marked this conversation as resolved.
Show resolved Hide resolved
// +optional
// NodeBalancerFirewallRef is a reference to a NodeBalancer Firewall object. This makes the linode use the specified NodeBalancer Firewall.
NodeBalancerFirewallRef *corev1.ObjectReference `json:"nodeBalancerFirewallRef,omitempty"`

eljohnson92 marked this conversation as resolved.
Show resolved Hide resolved
// CredentialsRef is a reference to a Secret that contains the credentials to use for provisioning this cluster. If not
// supplied then the credentials of the controller will be used.
// +optional
Expand Down Expand Up @@ -142,6 +147,9 @@ type NetworkSpec struct {
// NodeBalancerID is the id of NodeBalancer.
// +optional
NodeBalancerID *int `json:"nodeBalancerID,omitempty"`
// NodeBalancerFirewallID is the id of NodeBalancer Firewall.
// +optional
NodeBalancerFirewallID *int `json:"nodeBalancerFirewallID,omitempty"`
// apiserverNodeBalancerConfigID is the config ID of api server NodeBalancer config.
// +optional
ApiserverNodeBalancerConfigID *int `json:"apiserverNodeBalancerConfigID,omitempty"`
Expand Down
10 changes: 10 additions & 0 deletions api/v1alpha2/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion cloud/scope/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ func addCredentialsFinalizer(ctx context.Context, crClient K8sClient, credential
func removeCredentialsFinalizer(ctx context.Context, crClient K8sClient, credentialsRef corev1.SecretReference, defaultNamespace, finalizer string) error {
secret, err := getCredentials(ctx, crClient, credentialsRef, defaultNamespace)
if err != nil {
return err
return client.IgnoreNotFound(err)
}

controllerutil.RemoveFinalizer(secret, finalizer)
Expand Down
19 changes: 13 additions & 6 deletions cloud/scope/firewall.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"errors"
"fmt"

"k8s.io/client-go/util/retry"
"sigs.k8s.io/cluster-api/util/patch"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"

Expand Down Expand Up @@ -102,19 +103,25 @@ func (s *FirewallScope) AddCredentialsRefFinalizer(ctx context.Context) error {
return nil
}

return addCredentialsFinalizer(ctx, s.Client,
*s.LinodeFirewall.Spec.CredentialsRef, s.LinodeFirewall.GetNamespace(),
toFinalizer(s.LinodeFirewall))
// Retry on conflict to handle the case where the secret is being updated concurrently.
return retry.RetryOnConflict(retry.DefaultRetry, func() error {
return addCredentialsFinalizer(ctx, s.Client,
*s.LinodeFirewall.Spec.CredentialsRef, s.LinodeFirewall.GetNamespace(),
toFinalizer(s.LinodeFirewall))
})
}

func (s *FirewallScope) RemoveCredentialsRefFinalizer(ctx context.Context) error {
if s.LinodeFirewall.Spec.CredentialsRef == nil {
return nil
}

return removeCredentialsFinalizer(ctx, s.Client,
*s.LinodeFirewall.Spec.CredentialsRef, s.LinodeFirewall.GetNamespace(),
toFinalizer(s.LinodeFirewall))
// Retry on conflict to handle the case where the secret is being updated concurrently.
return retry.RetryOnConflict(retry.DefaultRetry, func() error {
return removeCredentialsFinalizer(ctx, s.Client,
*s.LinodeFirewall.Spec.CredentialsRef, s.LinodeFirewall.GetNamespace(),
toFinalizer(s.LinodeFirewall))
})
}

func (s *FirewallScope) SetCredentialRefTokenForLinodeClients(ctx context.Context) error {
Expand Down
56 changes: 56 additions & 0 deletions cloud/services/loadbalancers.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@

"github.com/go-logr/logr"
"github.com/linode/linodego"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/cluster-api/api/v1beta1"
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/linode/cluster-api-provider-linode/api/v1alpha2"
"github.com/linode/cluster-api-provider-linode/cloud/scope"
Expand All @@ -35,15 +37,69 @@
}

logger.Info(fmt.Sprintf("Creating NodeBalancer %s", clusterScope.LinodeCluster.Name))

createConfig := linodego.NodeBalancerCreateOptions{
Label: util.Pointer(clusterScope.LinodeCluster.Name),
Region: clusterScope.LinodeCluster.Spec.Region,
Tags: []string{string(clusterScope.LinodeCluster.UID)},
}

// get firewall ID from firewallRef if it exists
if clusterScope.LinodeCluster.Spec.NodeBalancerFirewallRef != nil {
komer3 marked this conversation as resolved.
Show resolved Hide resolved
firewallID, err := getFirewallID(ctx, clusterScope, logger)
if err != nil {
logger.Error(err, "Failed to fetch LinodeFirewall ID")
return nil, err
}
createConfig.FirewallID = firewallID
clusterScope.LinodeCluster.Spec.Network.NodeBalancerFirewallID = &firewallID
}

// Use a firewall created outside of the CAPL ecosystem
// get & validate firewall ID from .Spec.Network.FirewallID if it exists
if clusterScope.LinodeCluster.Spec.Network.NodeBalancerFirewallID != nil {
firewallID := *clusterScope.LinodeCluster.Spec.Network.NodeBalancerFirewallID
firewall, err := clusterScope.LinodeClient.GetFirewall(ctx, firewallID)
if err != nil {
logger.Error(err, "Failed to fetch Linode Firewall from the Linode API")
return nil, err
}
createConfig.FirewallID = firewall.ID
}

return clusterScope.LinodeClient.CreateNodeBalancer(ctx, createConfig)
}

func getFirewallID(ctx context.Context, clusterScope *scope.ClusterScope, logger logr.Logger) (int, error) {
name := clusterScope.LinodeCluster.Spec.NodeBalancerFirewallRef.Name
namespace := clusterScope.LinodeCluster.Spec.NodeBalancerFirewallRef.Namespace
if namespace == "" {
namespace = clusterScope.LinodeCluster.Namespace
}

Check warning on line 78 in cloud/services/loadbalancers.go

View check run for this annotation

Codecov / codecov/patch

cloud/services/loadbalancers.go#L77-L78

Added lines #L77 - L78 were not covered by tests

logger = logger.WithValues("firewallName", name, "firewallNamespace", namespace)

linodeFirewall := &v1alpha2.LinodeFirewall{
ObjectMeta: metav1.ObjectMeta{
Namespace: namespace,
Name: name,
},
}
objectKey := client.ObjectKeyFromObject(linodeFirewall)
err := clusterScope.Client.Get(ctx, objectKey, linodeFirewall)
if err != nil {
logger.Error(err, "Failed to fetch LinodeFirewall")
return -1, err
}
if linodeFirewall.Spec.FirewallID == nil {
err = errors.New("nil firewallID")
logger.Error(err, "Failed to fetch LinodeFirewall")
return -1, err
}

Check warning on line 98 in cloud/services/loadbalancers.go

View check run for this annotation

Codecov / codecov/patch

cloud/services/loadbalancers.go#L95-L98

Added lines #L95 - L98 were not covered by tests

return *linodeFirewall.Spec.FirewallID, nil
}

// EnsureNodeBalancerConfigs creates NodeBalancer configs if it does not exist or returns the existing NodeBalancerConfig
func EnsureNodeBalancerConfigs(
ctx context.Context,
Expand Down
Loading
Loading