Skip to content

Commit

Permalink
deprecate Identifiable and remove unused implementations (#1973)
Browse files Browse the repository at this point in the history
  • Loading branch information
s-urbaniak authored Dec 6, 2024
1 parent 30b82ed commit 4a5bc72
Show file tree
Hide file tree
Showing 11 changed files with 72 additions and 96 deletions.
34 changes: 20 additions & 14 deletions internal/set/identifiable.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,35 +4,41 @@ import (
"reflect"
)

// Identifiable is a simple interface wrapping any object which has some key field which can be used for later
// DeprecatedIdentifiable is a simple interface wrapping any object which has some key field which can be used for later
// aggregation operations (grouping, intersection, difference etc)
type Identifiable interface {
//
// Note: this construct is DEPRECATED. Instead, use the translation layer for comparing types.
type DeprecatedIdentifiable interface {
Identifier() interface{}
}

// Difference returns all 'Identifiable' elements that are in left slice and not in the right one.
// DeprecatedDifference returns all 'Identifiable' elements that are in left slice and not in the right one.
// Note, that despite the parameters being declared as 'interface{}' they must contain only the elements implementing
// 'Identifiable' interface (this is needed to solve lack of covariance in Go).
func Difference(left, right interface{}) []Identifiable {
//
// Note: this construct is DEPRECATED. Instead, use the translation layer for comparing types.
func DeprecatedDifference(left, right interface{}) []DeprecatedIdentifiable {
leftIdentifiers := toIdentifiableSlice(left)
rightIdentifiers := toIdentifiableSlice(right)

return differenceIdentifiable(leftIdentifiers, rightIdentifiers)
}

// Intersection returns all 'Identifiable' elements from 'left' and 'right' slice that intersect by 'Identifier()' value.
// DeprecatedIntersection returns all 'Identifiable' elements from 'left' and 'right' slice that intersect by 'Identifier()' value.
// Each intersection is represented as a tuple of two elements - matching elements from 'left' and 'right'.
// Note, that despite the parameters being declared as 'interface{}' they must contain only the elements implementing
// 'Identifiable' interface (this is needed to solve lack of covariance in Go)
func Intersection(left, right interface{}) [][]Identifiable {
//
// Note: this construct is DEPRECATED. Instead, use the translation layer for comparing types.
func DeprecatedIntersection(left, right interface{}) [][]DeprecatedIdentifiable {
leftIdentifiers := toIdentifiableSlice(left)
rightIdentifiers := toIdentifiableSlice(right)

return intersectionIdentifiable(leftIdentifiers, rightIdentifiers)
}

func differenceIdentifiable(left, right []Identifiable) []Identifiable {
result := make([]Identifiable, 0)
func differenceIdentifiable(left, right []DeprecatedIdentifiable) []DeprecatedIdentifiable {
result := make([]DeprecatedIdentifiable, 0)
for _, l := range left {
found := false
for _, r := range right {
Expand All @@ -48,25 +54,25 @@ func differenceIdentifiable(left, right []Identifiable) []Identifiable {
return result
}

func intersectionIdentifiable(left, right []Identifiable) [][]Identifiable {
result := make([][]Identifiable, 0)
func intersectionIdentifiable(left, right []DeprecatedIdentifiable) [][]DeprecatedIdentifiable {
result := make([][]DeprecatedIdentifiable, 0)
for _, l := range left {
for _, r := range right {
if r.Identifier() == l.Identifier() {
result = append(result, []Identifiable{l, r})
result = append(result, []DeprecatedIdentifiable{l, r})
}
}
}
return result
}

// toIdentifiableSlice uses reflection to cast the array
func toIdentifiableSlice(data interface{}) []Identifiable {
func toIdentifiableSlice(data interface{}) []DeprecatedIdentifiable {
value := reflect.ValueOf(data)

result := make([]Identifiable, value.Len())
result := make([]DeprecatedIdentifiable, value.Len())
for i := 0; i < value.Len(); i++ {
result[i] = value.Index(i).Interface().(Identifiable)
result[i] = value.Index(i).Interface().(DeprecatedIdentifiable)
}
return result
}
54 changes: 27 additions & 27 deletions internal/set/identifiable_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,19 +32,19 @@ func Test_SetDifference(t *testing.T) {
fourRight := newSome("4", "right")

testCases := []struct {
left []Identifiable
right []Identifiable
out []Identifiable
left []DeprecatedIdentifiable
right []DeprecatedIdentifiable
out []DeprecatedIdentifiable
}{
{left: []Identifiable{oneLeft, twoLeft}, right: []Identifiable{twoRight, threeRight}, out: []Identifiable{oneLeft}},
{left: []Identifiable{twoRight, threeRight}, right: []Identifiable{oneLeft, twoLeft}, out: []Identifiable{threeRight}},
{left: []Identifiable{oneLeft, twoLeft}, right: []Identifiable{threeRight, fourRight}, out: []Identifiable{oneLeft, twoLeft}},
{left: []DeprecatedIdentifiable{oneLeft, twoLeft}, right: []DeprecatedIdentifiable{twoRight, threeRight}, out: []DeprecatedIdentifiable{oneLeft}},
{left: []DeprecatedIdentifiable{twoRight, threeRight}, right: []DeprecatedIdentifiable{oneLeft, twoLeft}, out: []DeprecatedIdentifiable{threeRight}},
{left: []DeprecatedIdentifiable{oneLeft, twoLeft}, right: []DeprecatedIdentifiable{threeRight, fourRight}, out: []DeprecatedIdentifiable{oneLeft, twoLeft}},
// Empty
{left: []Identifiable{}, right: []Identifiable{threeRight, fourRight}, out: []Identifiable{}},
{left: []Identifiable{threeRight, fourRight}, right: []Identifiable{}, out: []Identifiable{threeRight, fourRight}},
{left: []DeprecatedIdentifiable{}, right: []DeprecatedIdentifiable{threeRight, fourRight}, out: []DeprecatedIdentifiable{}},
{left: []DeprecatedIdentifiable{threeRight, fourRight}, right: []DeprecatedIdentifiable{}, out: []DeprecatedIdentifiable{threeRight, fourRight}},
// Nil
{left: nil, right: []Identifiable{threeRight, fourRight}, out: []Identifiable{}},
{left: []Identifiable{threeRight, fourRight}, right: nil, out: []Identifiable{threeRight, fourRight}},
{left: nil, right: []DeprecatedIdentifiable{threeRight, fourRight}, out: []DeprecatedIdentifiable{}},
{left: []DeprecatedIdentifiable{threeRight, fourRight}, right: nil, out: []DeprecatedIdentifiable{threeRight, fourRight}},
}

for _, testCase := range testCases {
Expand All @@ -64,8 +64,8 @@ func Test_SetDifferenceCovariant(t *testing.T) {
leftNotIdentifiable := []someID{oneLeft, twoLeft}
rightNotIdentifiable := []someID{twoRight, threeRight}

assert.Equal(t, []Identifiable{oneLeft}, Difference(leftNotIdentifiable, rightNotIdentifiable))
assert.Equal(t, []Identifiable{threeRight}, Difference(rightNotIdentifiable, leftNotIdentifiable))
assert.Equal(t, []DeprecatedIdentifiable{oneLeft}, DeprecatedDifference(leftNotIdentifiable, rightNotIdentifiable))
assert.Equal(t, []DeprecatedIdentifiable{threeRight}, DeprecatedDifference(rightNotIdentifiable, leftNotIdentifiable))
}

func Test_SetIntersection(t *testing.T) {
Expand All @@ -76,22 +76,22 @@ func Test_SetIntersection(t *testing.T) {
fourRight := newSome("4", "right")

testCases := []struct {
left []Identifiable
right []Identifiable
out [][]Identifiable
left []DeprecatedIdentifiable
right []DeprecatedIdentifiable
out [][]DeprecatedIdentifiable
}{
// intersectionIdentifiable on "2"
{left: []Identifiable{oneLeft, twoLeft}, right: []Identifiable{twoRight, threeRight}, out: [][]Identifiable{pair(twoLeft, twoRight)}},
{left: []Identifiable{twoRight, threeRight}, right: []Identifiable{oneLeft, twoLeft}, out: [][]Identifiable{pair(twoRight, twoLeft)}},
{left: []DeprecatedIdentifiable{oneLeft, twoLeft}, right: []DeprecatedIdentifiable{twoRight, threeRight}, out: [][]DeprecatedIdentifiable{pair(twoLeft, twoRight)}},
{left: []DeprecatedIdentifiable{twoRight, threeRight}, right: []DeprecatedIdentifiable{oneLeft, twoLeft}, out: [][]DeprecatedIdentifiable{pair(twoRight, twoLeft)}},
// No intersection
{left: []Identifiable{oneLeft, twoLeft}, right: []Identifiable{threeRight, fourRight}, out: [][]Identifiable{}},
{left: []Identifiable{threeRight, fourRight}, right: []Identifiable{oneLeft, twoLeft}, out: [][]Identifiable{}},
{left: []DeprecatedIdentifiable{oneLeft, twoLeft}, right: []DeprecatedIdentifiable{threeRight, fourRight}, out: [][]DeprecatedIdentifiable{}},
{left: []DeprecatedIdentifiable{threeRight, fourRight}, right: []DeprecatedIdentifiable{oneLeft, twoLeft}, out: [][]DeprecatedIdentifiable{}},
// Empty
{left: []Identifiable{}, right: []Identifiable{threeRight, fourRight}, out: [][]Identifiable{}},
{left: []Identifiable{threeRight, fourRight}, right: []Identifiable{}, out: [][]Identifiable{}},
{left: []DeprecatedIdentifiable{}, right: []DeprecatedIdentifiable{threeRight, fourRight}, out: [][]DeprecatedIdentifiable{}},
{left: []DeprecatedIdentifiable{threeRight, fourRight}, right: []DeprecatedIdentifiable{}, out: [][]DeprecatedIdentifiable{}},
// Nil
{left: nil, right: []Identifiable{threeRight, fourRight}, out: [][]Identifiable{}},
{left: []Identifiable{threeRight, fourRight}, right: nil, out: [][]Identifiable{}},
{left: nil, right: []DeprecatedIdentifiable{threeRight, fourRight}, out: [][]DeprecatedIdentifiable{}},
{left: []DeprecatedIdentifiable{threeRight, fourRight}, right: nil, out: [][]DeprecatedIdentifiable{}},
}

for _, testCase := range testCases {
Expand All @@ -113,10 +113,10 @@ func Test_SetIntersectionCovariant(t *testing.T) {
leftNotIdentifiable := []someID{oneLeft, twoLeft}
rightNotIdentifiable := []someID{oneRight, twoRight, threeRight}

assert.Equal(t, [][]Identifiable{pair(oneLeft, oneRight), pair(twoLeft, twoRight)}, Intersection(leftNotIdentifiable, rightNotIdentifiable))
assert.Equal(t, [][]Identifiable{pair(oneRight, oneLeft), pair(twoRight, twoLeft)}, Intersection(rightNotIdentifiable, leftNotIdentifiable))
assert.Equal(t, [][]DeprecatedIdentifiable{pair(oneLeft, oneRight), pair(twoLeft, twoRight)}, DeprecatedIntersection(leftNotIdentifiable, rightNotIdentifiable))
assert.Equal(t, [][]DeprecatedIdentifiable{pair(oneRight, oneLeft), pair(twoRight, twoLeft)}, DeprecatedIntersection(rightNotIdentifiable, leftNotIdentifiable))
}

func pair(left, right Identifiable) []Identifiable {
return []Identifiable{left, right}
func pair(left, right DeprecatedIdentifiable) []DeprecatedIdentifiable {
return []DeprecatedIdentifiable{left, right}
}
4 changes: 0 additions & 4 deletions pkg/api/v1/atlasdatafederation_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,10 +119,6 @@ type DataFederationPE struct {
Type string `json:"type,omitempty"`
}

func (pe DataFederationPE) Identifier() interface{} {
return pe.EndpointID
}

var _ api.AtlasCustomResource = &AtlasDataFederation{}

// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
Expand Down
7 changes: 1 addition & 6 deletions pkg/api/v1/atlasteam_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,11 @@ package v1

import (
"go.mongodb.org/atlas-sdk/v20231115008/admin"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/compat"
"github.com/mongodb/mongodb-atlas-kubernetes/v2/pkg/api"
"github.com/mongodb/mongodb-atlas-kubernetes/v2/pkg/api/v1/status"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

var _ api.AtlasCustomResource = &AtlasTeam{}
Expand Down Expand Up @@ -69,10 +68,6 @@ func init() {
SchemeBuilder.Register(&AtlasTeam{}, &AtlasTeamList{})
}

func (in *AtlasTeam) Identifier() interface{} {
return in.Status.ID
}

func (in *AtlasTeam) GetStatus() api.Status {
return in.Status
}
Expand Down
3 changes: 1 addition & 2 deletions pkg/api/v1/project/integration.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,9 @@ import (
"context"

"go.mongodb.org/atlas/mongodbatlas"
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/mongodb/mongodb-atlas-kubernetes/v2/pkg/api/v1/common"

"sigs.k8s.io/controller-runtime/pkg/client"
)

type Integration struct {
Expand Down
12 changes: 0 additions & 12 deletions pkg/api/v1/project/ipaccesslist.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package project

import (
"strings"

"go.mongodb.org/atlas/mongodbatlas"

"github.com/mongodb/mongodb-atlas-kubernetes/v2/internal/compat"
Expand Down Expand Up @@ -33,16 +31,6 @@ func (i IPAccessList) ToAtlas() (*mongodbatlas.ProjectIPAccessList, error) {
return result, err
}

// Identifier returns the "id" of the ProjectIPAccessList. Note, that it's an error to specify more than one of these
// fields - the business layer must validate this beforehand
func (i IPAccessList) Identifier() interface{} {
if strings.Contains(i.CIDRBlock, "/32") {
cidrBlock := strings.Replace(i.CIDRBlock, "/32", "", 1) // if CIDRBlock is /32, then it's an IP address
return cidrBlock + i.AwsSecurityGroup + i.IPAddress
}
return i.CIDRBlock + i.AwsSecurityGroup + i.IPAddress
}

// ************************************ Builder methods *************************************************
// Note, that we don't use pointers here as the AtlasProject uses this without pointers

Expand Down
4 changes: 0 additions & 4 deletions pkg/api/v1/status/privateendpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,6 @@ type GCPEndpoint struct {
IPAddress string `json:"ipAddress"`
}

func (pe ProjectPrivateEndpoint) Identifier() interface{} {
return string(pe.Provider) + TransformRegionToID(pe.Region)
}

// TransformRegionToID makes the same ID from region and regionName fields for PE Connections to match them
// it leaves only characters which are letters or numbers starting from 2
// it also makes a couple swaps and sorts the resulting string
Expand Down
4 changes: 0 additions & 4 deletions pkg/controller/atlasproject/cloud_provider_integration.go
Original file line number Diff line number Diff line change
Expand Up @@ -303,10 +303,6 @@ func deleteCloudProviderAccess(workflowCtx *workflow.Context, projectID string,

type CloudProviderIntegrationIdentifiable akov2.CloudProviderIntegration

func (cpa CloudProviderIntegrationIdentifiable) Identifier() interface{} {
return fmt.Sprintf("%s.%s", cpa.ProviderName, cpa.IamAssumedRoleArn)
}

func convertCloudProviderAccessRole(cpa admin.CloudProviderAccessRole) admin.CloudProviderAccessAWSIAMRole {
return admin.CloudProviderAccessAWSIAMRole{
Id: cpa.Id,
Expand Down
18 changes: 9 additions & 9 deletions pkg/controller/atlasproject/integrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,19 +38,19 @@ func (r *AtlasProjectReconciler) createOrDeleteIntegrations(ctx *workflow.Contex
}
integrationsInAtlasAlias := toAliasThirdPartyIntegration(integrationsInAtlas.Results)

identifiersForDelete := set.Difference(integrationsInAtlasAlias, project.Spec.Integrations)
identifiersForDelete := set.DeprecatedDifference(integrationsInAtlasAlias, project.Spec.Integrations)
ctx.Log.Debugf("identifiersForDelete: %v", identifiersForDelete)
if err := deleteIntegrationsFromAtlas(ctx, projectID, identifiersForDelete); err != nil {
return workflow.Terminate(workflow.ProjectIntegrationInternal, err.Error())
}

integrationsToUpdate := set.Intersection(integrationsInAtlasAlias, project.Spec.Integrations)
integrationsToUpdate := set.DeprecatedIntersection(integrationsInAtlasAlias, project.Spec.Integrations)
ctx.Log.Debugf("integrationsToUpdate: %v", integrationsToUpdate)
if result := r.updateIntegrationsAtlas(ctx, projectID, integrationsToUpdate, project.Namespace); !result.IsOk() {
return result
}

identifiersForCreate := set.Difference(project.Spec.Integrations, integrationsInAtlasAlias)
identifiersForCreate := set.DeprecatedDifference(project.Spec.Integrations, integrationsInAtlasAlias)
ctx.Log.Debugf("identifiersForCreate: %v", identifiersForCreate)
if result := r.createIntegrationsInAtlas(ctx, projectID, identifiersForCreate, project.Namespace); !result.IsOk() {
return result
Expand All @@ -73,7 +73,7 @@ func fetchIntegrations(ctx *workflow.Context, projectID string) (*mongodbatlas.T
return integrationsInAtlas, nil
}

func (r *AtlasProjectReconciler) updateIntegrationsAtlas(ctx *workflow.Context, projectID string, integrationsToUpdate [][]set.Identifiable, namespace string) workflow.Result {
func (r *AtlasProjectReconciler) updateIntegrationsAtlas(ctx *workflow.Context, projectID string, integrationsToUpdate [][]set.DeprecatedIdentifiable, namespace string) workflow.Result {
for _, item := range integrationsToUpdate {
kubeIntegration, err := item[1].(project.Integration).ToAtlas(ctx.Context, r.Client, namespace)
if kubeIntegration == nil {
Expand All @@ -90,7 +90,7 @@ func (r *AtlasProjectReconciler) updateIntegrationsAtlas(ctx *workflow.Context,
return workflow.OK()
}

func deleteIntegrationsFromAtlas(ctx *workflow.Context, projectID string, integrationsToRemove []set.Identifiable) error {
func deleteIntegrationsFromAtlas(ctx *workflow.Context, projectID string, integrationsToRemove []set.DeprecatedIdentifiable) error {
for _, integration := range integrationsToRemove {
if _, err := ctx.Client.Integrations.Delete(ctx.Context, projectID, integration.Identifier().(string)); err != nil {
return err
Expand All @@ -100,7 +100,7 @@ func deleteIntegrationsFromAtlas(ctx *workflow.Context, projectID string, integr
return nil
}

func (r *AtlasProjectReconciler) createIntegrationsInAtlas(ctx *workflow.Context, projectID string, integrations []set.Identifiable, namespace string) workflow.Result {
func (r *AtlasProjectReconciler) createIntegrationsInAtlas(ctx *workflow.Context, projectID string, integrations []set.DeprecatedIdentifiable, namespace string) workflow.Result {
for _, item := range integrations {
integration, err := item.(project.Integration).ToAtlas(ctx.Context, r.Client, namespace)
if err != nil || integration == nil {
Expand All @@ -118,7 +118,7 @@ func (r *AtlasProjectReconciler) createIntegrationsInAtlas(ctx *workflow.Context
return workflow.OK()
}

func (r *AtlasProjectReconciler) checkIntegrationsReady(ctx *workflow.Context, namespace string, integrationsIntersection [][]set.Identifiable, requestedIntegrations []project.Integration) bool {
func (r *AtlasProjectReconciler) checkIntegrationsReady(ctx *workflow.Context, namespace string, integrationsIntersection [][]set.DeprecatedIdentifiable, requestedIntegrations []project.Integration) bool {
if len(integrationsIntersection) != len(requestedIntegrations) {
return false
}
Expand Down Expand Up @@ -163,7 +163,7 @@ func toAliasThirdPartyIntegration(list []*mongodbatlas.ThirdPartyIntegration) []
return result
}

func syncPrometheusStatus(ctx *workflow.Context, project *akov2.AtlasProject, integrationPairs [][]set.Identifiable) {
func syncPrometheusStatus(ctx *workflow.Context, project *akov2.AtlasProject, integrationPairs [][]set.DeprecatedIdentifiable) {
prometheusIntegration, found := searchAtlasIntegration(integrationPairs, isPrometheusType)
if !found {
ctx.EnsureStatusOption(status.AtlasProjectPrometheusOption(nil))
Expand All @@ -176,7 +176,7 @@ func syncPrometheusStatus(ctx *workflow.Context, project *akov2.AtlasProject, in
}))
}

func searchAtlasIntegration(integrationPairs [][]set.Identifiable, filterFunc func(typeName string) bool) (integration mongodbatlas.ThirdPartyIntegration, found bool) {
func searchAtlasIntegration(integrationPairs [][]set.DeprecatedIdentifiable, filterFunc func(typeName string) bool) (integration mongodbatlas.ThirdPartyIntegration, found bool) {
for _, pair := range integrationPairs {
integrationAlias := pair[0].(aliasThirdPartyIntegration)
if filterFunc(integrationAlias.Type) {
Expand Down
Loading

0 comments on commit 4a5bc72

Please sign in to comment.