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

🌱 IdentityRefProvider: DRY obtaining OpenStackIdentityRef #2132

Merged
merged 1 commit into from
Jun 24, 2024
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
7 changes: 7 additions & 0 deletions api/v1alpha1/openstackfloatingippool_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,13 @@ func (r *OpenStackFloatingIPPool) GetFloatingIPTag() string {
return fmt.Sprintf("cluster-api-provider-openstack-fip-pool-%s", r.Name)
}

var _ infrav1.IdentityRefProvider = &OpenStackFloatingIPPool{}

// GetIdentifyRef returns the FloatingIPPool's namespace and IdentityRef.
func (r *OpenStackFloatingIPPool) GetIdentityRef() (*string, *infrav1.OpenStackIdentityReference) {
return &r.Namespace, &r.Spec.IdentityRef
}

func init() {
SchemeBuilder.Register(&OpenStackFloatingIPPool{}, &OpenStackFloatingIPPoolList{})
}
7 changes: 7 additions & 0 deletions api/v1beta1/identity_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,10 @@ type OpenStackIdentityReference struct {
// +kubebuilder:validation:Required
CloudName string `json:"cloudName"`
}

// IdentityRefProvider is an interface for obtaining OpenStack credentials from an API object
// +kubebuilder:object:generate:=false
type IdentityRefProvider interface {
// GetIdentifyRef returns the object's namespace and IdentityRef if it has an IdentityRef, or nulls if it does not
GetIdentityRef() (*string, *OpenStackIdentityReference)
}
7 changes: 7 additions & 0 deletions api/v1beta1/openstackcluster_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,13 @@ type ManagedSecurityGroups struct {
AllowAllInClusterTraffic bool `json:"allowAllInClusterTraffic"`
}

var _ IdentityRefProvider = &OpenStackCluster{}

// GetIdentifyRef returns the cluster's namespace and IdentityRef.
func (c *OpenStackCluster) GetIdentityRef() (*string, *OpenStackIdentityReference) {
return &c.Namespace, &c.Spec.IdentityRef
}

func init() {
objectTypes = append(objectTypes, &OpenStackCluster{}, &OpenStackClusterList{})
}
10 changes: 10 additions & 0 deletions api/v1beta1/openstackmachine_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,16 @@ func (r *OpenStackMachine) SetFailure(failureReason errors.MachineStatusError, f
r.Status.FailureMessage = ptr.To(failureMessage.Error())
}

var _ IdentityRefProvider = &OpenStackMachine{}

// GetIdentifyRef returns the object's namespace and IdentityRef if it has an IdentityRef, or nulls if it does not.
func (r *OpenStackMachine) GetIdentityRef() (*string, *OpenStackIdentityReference) {
if r.Spec.IdentityRef != nil {
return &r.Namespace, r.Spec.IdentityRef
}
return nil, nil
}

func init() {
objectTypes = append(objectTypes, &OpenStackMachine{}, &OpenStackMachineList{})
}
2 changes: 1 addition & 1 deletion controllers/openstackcluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func (r *OpenStackClusterReconciler) Reconcile(ctx context.Context, req ctrl.Req
}
}()

clientScope, err := r.ScopeFactory.NewClientScopeFromCluster(ctx, r.Client, openStackCluster, r.CaCertificates, log)
clientScope, err := r.ScopeFactory.NewClientScopeFromObject(ctx, r.Client, r.CaCertificates, log, openStackCluster)
if err != nil {
return reconcile.Result{}, err
}
Expand Down
16 changes: 8 additions & 8 deletions controllers/openstackcluster_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ var _ = Describe("OpenStackCluster controller", func() {
err = k8sClient.Status().Update(ctx, testCluster)
Expect(err).To(BeNil())
log := GinkgoLogr
clientScope, err := mockScopeFactory.NewClientScopeFromCluster(ctx, k8sClient, testCluster, nil, log)
clientScope, err := mockScopeFactory.NewClientScopeFromObject(ctx, k8sClient, nil, log, testCluster)
Expect(err).To(BeNil())
scope := scope.NewWithLogger(clientScope, log)

Expand Down Expand Up @@ -268,7 +268,7 @@ var _ = Describe("OpenStackCluster controller", func() {
Expect(err).To(BeNil())

log := GinkgoLogr
clientScope, err := mockScopeFactory.NewClientScopeFromCluster(ctx, k8sClient, testCluster, nil, log)
clientScope, err := mockScopeFactory.NewClientScopeFromObject(ctx, k8sClient, nil, log, testCluster)
Expect(err).To(BeNil())
scope := scope.NewWithLogger(clientScope, log)

Expand Down Expand Up @@ -352,7 +352,7 @@ var _ = Describe("OpenStackCluster controller", func() {
Expect(err).To(BeNil())

log := GinkgoLogr
clientScope, err := mockScopeFactory.NewClientScopeFromCluster(ctx, k8sClient, testCluster, nil, log)
clientScope, err := mockScopeFactory.NewClientScopeFromObject(ctx, k8sClient, nil, log, testCluster)
Expect(err).To(BeNil())
scope := scope.NewWithLogger(clientScope, log)

Expand Down Expand Up @@ -434,7 +434,7 @@ var _ = Describe("OpenStackCluster controller", func() {
Expect(err).To(BeNil())

log := GinkgoLogr
clientScope, err := mockScopeFactory.NewClientScopeFromCluster(ctx, k8sClient, testCluster, nil, log)
clientScope, err := mockScopeFactory.NewClientScopeFromObject(ctx, k8sClient, nil, log, testCluster)
Expect(err).To(BeNil())
scope := scope.NewWithLogger(clientScope, log)

Expand Down Expand Up @@ -491,7 +491,7 @@ var _ = Describe("OpenStackCluster controller", func() {
Expect(err).To(BeNil())

log := GinkgoLogr
clientScope, err := mockScopeFactory.NewClientScopeFromCluster(ctx, k8sClient, testCluster, nil, log)
clientScope, err := mockScopeFactory.NewClientScopeFromObject(ctx, k8sClient, nil, log, testCluster)
Expect(err).To(BeNil())
scope := scope.NewWithLogger(clientScope, log)

Expand Down Expand Up @@ -546,7 +546,7 @@ var _ = Describe("OpenStackCluster controller", func() {
Expect(err).To(BeNil())

log := GinkgoLogr
clientScope, err := mockScopeFactory.NewClientScopeFromCluster(ctx, k8sClient, testCluster, nil, log)
clientScope, err := mockScopeFactory.NewClientScopeFromObject(ctx, k8sClient, nil, log, testCluster)
Expect(err).To(BeNil())
scope := scope.NewWithLogger(clientScope, log)

Expand Down Expand Up @@ -620,7 +620,7 @@ var _ = Describe("OpenStackCluster controller", func() {
Expect(err).To(BeNil())

log := GinkgoLogr
clientScope, err := mockScopeFactory.NewClientScopeFromCluster(ctx, k8sClient, testCluster, nil, log)
clientScope, err := mockScopeFactory.NewClientScopeFromObject(ctx, k8sClient, nil, log, testCluster)
Expect(err).To(BeNil())
scope := scope.NewWithLogger(clientScope, log)

Expand Down Expand Up @@ -676,7 +676,7 @@ var _ = Describe("OpenStackCluster controller", func() {
Expect(err).To(BeNil())

log := GinkgoLogr
clientScope, err := mockScopeFactory.NewClientScopeFromCluster(ctx, k8sClient, testCluster, nil, log)
clientScope, err := mockScopeFactory.NewClientScopeFromObject(ctx, k8sClient, nil, log, testCluster)
Expect(err).To(BeNil())
scope := scope.NewWithLogger(clientScope, log)

Expand Down
2 changes: 1 addition & 1 deletion controllers/openstackfloatingippool_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func (r *OpenStackFloatingIPPoolReconciler) Reconcile(ctx context.Context, req c
return ctrl.Result{}, client.IgnoreNotFound(err)
}

clientScope, err := r.ScopeFactory.NewClientScopeFromFloatingIPPool(ctx, r.Client, pool, r.CaCertificates, log)
clientScope, err := r.ScopeFactory.NewClientScopeFromObject(ctx, r.Client, r.CaCertificates, log, pool)
if err != nil {
return reconcile.Result{}, err
}
Expand Down
2 changes: 1 addition & 1 deletion controllers/openstackmachine_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ func (r *OpenStackMachineReconciler) Reconcile(ctx context.Context, req ctrl.Req
}
}()

clientScope, err := r.ScopeFactory.NewClientScopeFromMachine(ctx, r.Client, openStackMachine, infraCluster, r.CaCertificates, log)
clientScope, err := r.ScopeFactory.NewClientScopeFromObject(ctx, r.Client, r.CaCertificates, log, openStackMachine, infraCluster)
if err != nil {
return reconcile.Result{}, err
}
Expand Down
5 changes: 5 additions & 0 deletions docs/book/src/api/v1beta1/api.md

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

17 changes: 1 addition & 16 deletions pkg/scope/mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
"go.uber.org/mock/gomock"
"sigs.k8s.io/controller-runtime/pkg/client"

infrav1alpha1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha1"
infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1"
"sigs.k8s.io/cluster-api-provider-openstack/pkg/clients"
"sigs.k8s.io/cluster-api-provider-openstack/pkg/clients/mock"
Expand Down Expand Up @@ -65,21 +64,7 @@ func (f *MockScopeFactory) SetClientScopeCreateError(err error) {
f.clientScopeCreateError = err
}

func (f *MockScopeFactory) NewClientScopeFromMachine(_ context.Context, _ client.Client, _ *infrav1.OpenStackMachine, _ *infrav1.OpenStackCluster, _ []byte, _ logr.Logger) (Scope, error) {
if f.clientScopeCreateError != nil {
return nil, f.clientScopeCreateError
}
return f, nil
}

func (f *MockScopeFactory) NewClientScopeFromCluster(_ context.Context, _ client.Client, _ *infrav1.OpenStackCluster, _ []byte, _ logr.Logger) (Scope, error) {
if f.clientScopeCreateError != nil {
return nil, f.clientScopeCreateError
}
return f, nil
}

func (f *MockScopeFactory) NewClientScopeFromFloatingIPPool(_ context.Context, _ client.Client, _ *infrav1alpha1.OpenStackFloatingIPPool, _ []byte, _ logr.Logger) (Scope, error) {
func (f *MockScopeFactory) NewClientScopeFromObject(_ context.Context, _ client.Client, _ []byte, _ logr.Logger, _ ...infrav1.IdentityRefProvider) (Scope, error) {
if f.clientScopeCreateError != nil {
return nil, f.clientScopeCreateError
}
Expand Down
52 changes: 7 additions & 45 deletions pkg/scope/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/yaml"

infrav1alpha1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha1"
infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1"
"sigs.k8s.io/cluster-api-provider-openstack/pkg/clients"
"sigs.k8s.io/cluster-api-provider-openstack/pkg/utils/hash"
Expand All @@ -53,60 +52,23 @@ type providerScopeFactory struct {
clientCache *cache.LRUExpireCache
}

func (f *providerScopeFactory) NewClientScopeFromMachine(ctx context.Context, ctrlClient client.Client, openStackMachine *infrav1.OpenStackMachine, openStackCluster *infrav1.OpenStackCluster, defaultCACert []byte, logger logr.Logger) (Scope, error) {
var cloud clientconfig.Cloud
var caCert []byte

func (f *providerScopeFactory) NewClientScopeFromObject(ctx context.Context, ctrlClient client.Client, defaultCACert []byte, logger logr.Logger, objects ...infrav1.IdentityRefProvider) (Scope, error) {
var namespace *string
var identityRef *infrav1.OpenStackIdentityReference
var namespace string
if openStackMachine.Spec.IdentityRef != nil {
identityRef = openStackMachine.Spec.IdentityRef
namespace = openStackMachine.Namespace
} else {
identityRef = &openStackCluster.Spec.IdentityRef
namespace = openStackCluster.Namespace
}

var err error
cloud, caCert, err = getCloudFromSecret(ctx, ctrlClient, namespace, identityRef.Name, identityRef.CloudName)
if err != nil {
return nil, err
}

if caCert == nil {
caCert = defaultCACert
for _, o := range objects {
namespace, identityRef = o.GetIdentityRef()
}

if f.clientCache == nil {
return NewProviderScope(cloud, caCert, logger)
if namespace == nil || identityRef == nil {
return nil, fmt.Errorf("unable to get identityRef from provided objects")
}

return NewCachedProviderScope(f.clientCache, cloud, caCert, logger)
}

func (f *providerScopeFactory) NewClientScopeFromCluster(ctx context.Context, ctrlClient client.Client, openStackCluster *infrav1.OpenStackCluster, defaultCACert []byte, logger logr.Logger) (Scope, error) {
var cloud clientconfig.Cloud
var caCert []byte

var err error
cloud, caCert, err = getCloudFromSecret(ctx, ctrlClient, openStackCluster.Namespace, openStackCluster.Spec.IdentityRef.Name, openStackCluster.Spec.IdentityRef.CloudName)
if err != nil {
return nil, err
}

if caCert == nil {
caCert = defaultCACert
}

if f.clientCache == nil {
return NewProviderScope(cloud, caCert, logger)
}

return NewCachedProviderScope(f.clientCache, cloud, caCert, logger)
}

func (f *providerScopeFactory) NewClientScopeFromFloatingIPPool(ctx context.Context, ctrlClient client.Client, openstackFloatingIPPool *infrav1alpha1.OpenStackFloatingIPPool, defaultCACert []byte, logger logr.Logger) (Scope, error) {
cloud, caCert, err := getCloudFromSecret(ctx, ctrlClient, openstackFloatingIPPool.Namespace, openstackFloatingIPPool.Spec.IdentityRef.Name, openstackFloatingIPPool.Spec.IdentityRef.CloudName)
cloud, caCert, err = getCloudFromSecret(ctx, ctrlClient, *namespace, identityRef.Name, identityRef.CloudName)
if err != nil {
return nil, err
}
Expand Down
8 changes: 3 additions & 5 deletions pkg/scope/scope.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
"k8s.io/apimachinery/pkg/util/cache"
"sigs.k8s.io/controller-runtime/pkg/client"

infrav1alpha1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha1"
infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1"
"sigs.k8s.io/cluster-api-provider-openstack/pkg/clients"
)
Expand All @@ -40,11 +39,10 @@ func NewFactory(maxCacheSize int) Factory {
}
}

// Factory instantiates a new Scope using credentials from either a cluster or a machine.
// Factory instantiates a new Scope using credentials from an IdentityRefProvider.
type Factory interface {
NewClientScopeFromMachine(ctx context.Context, ctrlClient client.Client, openStackMachine *infrav1.OpenStackMachine, openStackCluster *infrav1.OpenStackCluster, defaultCACert []byte, logger logr.Logger) (Scope, error)
NewClientScopeFromCluster(ctx context.Context, ctrlClient client.Client, openStackCluster *infrav1.OpenStackCluster, defaultCACert []byte, logger logr.Logger) (Scope, error)
NewClientScopeFromFloatingIPPool(ctx context.Context, ctrlClient client.Client, openStackCluster *infrav1alpha1.OpenStackFloatingIPPool, defaultCACert []byte, logger logr.Logger) (Scope, error)
// NewClientScopeFromObject creates a new scope from the first object which returns an OpenStackIdentityRef
NewClientScopeFromObject(ctx context.Context, ctrlClient client.Client, defaultCACert []byte, logger logr.Logger, objects ...infrav1.IdentityRefProvider) (Scope, error)
}

// Scope contains arguments common to most operations.
Expand Down