Skip to content

Commit

Permalink
[NSXServiceAccount] Fix store issues (vmware-tanzu#642) (vmware-tanzu…
Browse files Browse the repository at this point in the history 
…#644)

[NSXServiceAccount] Fix store issues
  • Loading branch information
@gran-vmv
gran-vmv authored Jul 22, 2024
1 parent 48300e8 commit 56e0a6f
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 61 deletions.
8 changes: 4 additions & 4 deletions pkg/controllers/nsxserviceaccount/nsxserviceaccount_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -559,7 +559,7 @@ func TestNSXServiceAccountReconciler_GarbageCollector(t *testing.T) {
name2 := "name2"
clusterName2 := "cl1-ns2-name2"
uid2 := "00000000-0000-0000-0000-000000000002"
assert.NoError(t, r.Service.PrincipalIdentityStore.Add(mpmodel.PrincipalIdentity{
assert.NoError(t, r.Service.PrincipalIdentityStore.Add(&mpmodel.PrincipalIdentity{
Name: &clusterName2,
Tags: []mpmodel.Tag{{
Scope: &tagScopeNamespace,
Expand Down Expand Up @@ -818,7 +818,7 @@ func TestNSXServiceAccountReconciler_garbageCollector(t *testing.T) {
name2 := "name2"
clusterName2 := "cl1-ns2-name2"
uid2 := "00000000-0000-0000-0000-000000000002"
assert.NoError(t, r.Service.PrincipalIdentityStore.Add(mpmodel.PrincipalIdentity{
assert.NoError(t, r.Service.PrincipalIdentityStore.Add(&mpmodel.PrincipalIdentity{
Name: &clusterName2,
Tags: []mpmodel.Tag{{
Scope: &tagScopeNamespace,
Expand All @@ -835,7 +835,7 @@ func TestNSXServiceAccountReconciler_garbageCollector(t *testing.T) {
name3 := "name3"
clusterName3 := "cl1-ns3-name3"
uid3 := "00000000-0000-0000-0000-000000000003"
assert.NoError(t, r.Service.PrincipalIdentityStore.Add(mpmodel.PrincipalIdentity{
assert.NoError(t, r.Service.PrincipalIdentityStore.Add(&mpmodel.PrincipalIdentity{
Name: &clusterName3,
Tags: []mpmodel.Tag{{
Scope: &tagScopeNamespace,
Expand All @@ -852,7 +852,7 @@ func TestNSXServiceAccountReconciler_garbageCollector(t *testing.T) {
name4 := "name4"
clusterName4 := "cl1-ns4-name4"
uid4 := "00000000-0000-0000-0000-000000000004"
assert.NoError(t, r.Service.ClusterControlPlaneStore.Add(model.ClusterControlPlane{
assert.NoError(t, r.Service.ClusterControlPlaneStore.Add(&model.ClusterControlPlane{
Id: &clusterName4,
Tags: []model.Tag{{
Scope: &tagScopeNamespace,
Expand Down
4 changes: 3 additions & 1 deletion pkg/nsx/services/common/store.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,8 @@ func (service *Service) InitializeCommonStore(wg *sync.WaitGroup, fatalErrors ch
pathUnescape, _ := url.PathUnescape("path%3A")
queryParam += " AND " + pathUnescape + path
}
queryParam += " AND marked_for_delete:false"
if store.IsPolicyAPI() {
queryParam += " AND marked_for_delete:false"
}
service.PopulateResourcetoStore(wg, fatalErrors, resourceTypeValue, queryParam, store, nil)
}
22 changes: 12 additions & 10 deletions pkg/nsx/services/nsxserviceaccount/cluster.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ func (s *NSXServiceAccountService) createPIAndCCP(normalizedClusterName string,
if err != nil {
return "", err
}
s.PrincipalIdentityStore.Add(pi)
s.PrincipalIdentityStore.Add(&pi)
} else if !hasPI != (piObj == nil) {
return "", fmt.Errorf("old PI exists")
}
Expand All @@ -263,7 +263,7 @@ func (s *NSXServiceAccountService) createPIAndCCP(normalizedClusterName string,
if err != nil {
return "", err
}
s.ClusterControlPlaneStore.Add(ccp)
s.ClusterControlPlaneStore.Add(&ccp)
clusterId = *ccp.NodeId
} else if !hasCCP != (ccpObj == nil) {
return "", fmt.Errorf("old CCP exists")
Expand Down Expand Up @@ -335,12 +335,12 @@ func (s *NSXServiceAccountService) DeleteNSXServiceAccount(ctx context.Context,
log.Error(err, "failed to delete", "ClusterControlPlane", normalizedClusterName)
return err
}
s.ClusterControlPlaneStore.Delete(model.ClusterControlPlane{Id: &normalizedClusterName})
s.ClusterControlPlaneStore.Delete(&model.ClusterControlPlane{Id: &normalizedClusterName})
}

// delete PI
if piobj := s.PrincipalIdentityStore.GetByKey(normalizedClusterName); isDeletePI && (piobj != nil) {
pi := piobj.(mpmodel.PrincipalIdentity)
pi := piobj.(*mpmodel.PrincipalIdentity)
if err := s.NSXClient.PrincipalIdentitiesClient.Delete(*pi.Id); err != nil {
log.Error(err, "failed to delete", "PrincipalIdentity", *pi.Name)
return err
Expand Down Expand Up @@ -430,16 +430,17 @@ func (s *NSXServiceAccountService) updatePIAndCCPCert(normalizedClusterName, uid
}

// update ClusterControlPlane cert
ccp := ccpObj.(model.ClusterControlPlane)
ccp := ccpObj.(*model.ClusterControlPlane)
ccp.Certificate = &cert
if ccp, err := s.NSXClient.ClusterControlPlanesClient.Update(siteId, enforcementpointId, normalizedClusterName, ccp); err != nil {
if ccp2, err := s.NSXClient.ClusterControlPlanesClient.Update(siteId, enforcementpointId, normalizedClusterName, *ccp); err != nil {
return err
} else {
ccp = &ccp2
s.ClusterControlPlaneStore.Add(ccp)
}

// update PI cert
pi := piObj.(mpmodel.PrincipalIdentity)
pi := piObj.(*mpmodel.PrincipalIdentity)
oldCertId := ""
if pi.CertificateId != nil {
oldCertId = *pi.CertificateId
Expand All @@ -451,12 +452,13 @@ func (s *NSXServiceAccountService) updatePIAndCCPCert(normalizedClusterName, uid
if err != nil {
return err
}
if pi, err = s.NSXClient.PrincipalIdentitiesClient.Updatecertificate(mpmodel.UpdatePrincipalIdentityCertificateRequest{
if pi2, err := s.NSXClient.PrincipalIdentitiesClient.Updatecertificate(mpmodel.UpdatePrincipalIdentityCertificateRequest{
CertificateId: certList.Results[0].Id,
PrincipalIdentityId: pi.Id,
}); err != nil {
return err
} else {
pi = &pi2
s.PrincipalIdentityStore.Add(pi)
}
if oldCertId != "" {
Expand Down Expand Up @@ -484,7 +486,7 @@ func (s *NSXServiceAccountService) GetNSXServiceAccountNameByUID(uid string) (na
return
}
for _, obj := range objs {
pi := obj.(mpmodel.PrincipalIdentity)
pi := obj.(*mpmodel.PrincipalIdentity)
for _, tag := range pi.Tags {
switch *tag.Scope {
case common.TagScopeNamespace:
Expand All @@ -503,7 +505,7 @@ func (s *NSXServiceAccountService) GetNSXServiceAccountNameByUID(uid string) (na
return
}
for _, obj := range objs {
ccp := obj.(model.ClusterControlPlane)
ccp := obj.(*model.ClusterControlPlane)
for _, tag := range ccp.Tags {
if tag.Scope != nil {
switch *tag.Scope {
Expand Down
26 changes: 13 additions & 13 deletions pkg/nsx/services/nsxserviceaccount/cluster_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -551,7 +551,7 @@ func TestNSXServiceAccountService_RestoreRealizedNSXServiceAccount(t *testing.T)
vpcPath := "/orgs/default/projects/k8scl-one:test/vpcs/vpc1"
piId := "Id1"
uid := "00000000-0000-0000-0000-000000000001"
s.PrincipalIdentityStore.Add(mpmodel.PrincipalIdentity{
s.PrincipalIdentityStore.Add(&mpmodel.PrincipalIdentity{
IsProtected: &isProtectedTrue,
Name: &normalizedClusterName,
NodeId: &normalizedClusterName,
Expand Down Expand Up @@ -583,7 +583,7 @@ func TestNSXServiceAccountService_RestoreRealizedNSXServiceAccount(t *testing.T)
}},
})
nodeId := "clusterId1"
s.ClusterControlPlaneStore.Add(model.ClusterControlPlane{
s.ClusterControlPlaneStore.Add(&model.ClusterControlPlane{
Id: &normalizedClusterName,
NodeId: &nodeId,
Revision: &revision1,
Expand Down Expand Up @@ -637,7 +637,7 @@ func TestNSXServiceAccountService_RestoreRealizedNSXServiceAccount(t *testing.T)
vpcPath := "/orgs/default/projects/k8scl-one:test/vpcs/vpc1"
piId := "Id1"
uid := "00000000-0000-0000-0000-000000000001"
s.PrincipalIdentityStore.Add(mpmodel.PrincipalIdentity{
s.PrincipalIdentityStore.Add(&mpmodel.PrincipalIdentity{
IsProtected: &isProtectedTrue,
Name: &normalizedClusterName,
NodeId: &normalizedClusterName,
Expand Down Expand Up @@ -1130,8 +1130,8 @@ func TestNSXServiceAccountService_ValidateAndUpdateRealizedNSXServiceAccount(t *
Scope: &uidScope,
Tag: &uidTag,
}}}
assert.NoError(t, s.ClusterControlPlaneStore.Add(ccp))
assert.NoError(t, s.PrincipalIdentityStore.Add(pi))
assert.NoError(t, s.ClusterControlPlaneStore.Add(&ccp))
assert.NoError(t, s.PrincipalIdentityStore.Add(&pi))

patches := gomonkey.ApplyMethodSeq(s.NSXClient, "NSXCheckVersion", []gomonkey.OutputCell{{
Values: gomonkey.Params{true},
Expand Down Expand Up @@ -1245,8 +1245,8 @@ func TestNSXServiceAccountService_DeleteNSXServiceAccount(t *testing.T) {
normalizedClusterName := "k8scl-one_test-ns1-name1"
piId := "piId1"
certId := "certId1"
assert.NoError(t, s.ClusterControlPlaneStore.Add(model.ClusterControlPlane{Id: &normalizedClusterName}))
assert.NoError(t, s.PrincipalIdentityStore.Add(mpmodel.PrincipalIdentity{Name: &normalizedClusterName, Id: &piId, CertificateId: &certId}))
assert.NoError(t, s.ClusterControlPlaneStore.Add(&model.ClusterControlPlane{Id: &normalizedClusterName}))
assert.NoError(t, s.PrincipalIdentityStore.Add(&mpmodel.PrincipalIdentity{Name: &normalizedClusterName, Id: &piId, CertificateId: &certId}))
assert.NoError(t, s.Client.Create(ctx, &v1alpha1.NSXServiceAccount{
ObjectMeta: metav1.ObjectMeta{
Name: "name1",
Expand Down Expand Up @@ -1285,8 +1285,8 @@ func TestNSXServiceAccountService_DeleteNSXServiceAccount(t *testing.T) {
normalizedClusterName := "k8scl-one_test-ns1-name1"
piId := "piId1"
certId := "certId1"
assert.NoError(t, s.ClusterControlPlaneStore.Add(model.ClusterControlPlane{Id: &normalizedClusterName}))
assert.NoError(t, s.PrincipalIdentityStore.Add(mpmodel.PrincipalIdentity{Name: &normalizedClusterName, Id: &piId, CertificateId: &certId, Tags: []mpmodel.Tag{{
assert.NoError(t, s.ClusterControlPlaneStore.Add(&model.ClusterControlPlane{Id: &normalizedClusterName}))
assert.NoError(t, s.PrincipalIdentityStore.Add(&mpmodel.PrincipalIdentity{Name: &normalizedClusterName, Id: &piId, CertificateId: &certId, Tags: []mpmodel.Tag{{
Scope: &uidScope,
Tag: &uidTag,
}}}))
Expand Down Expand Up @@ -1357,7 +1357,7 @@ func TestNSXServiceAccountService_ListNSXServiceAccountRealization(t *testing.T)
piName := piKey
piId := piKey + "-id"
crUID := piKey + "-uid"
assert.NoError(t, s.PrincipalIdentityStore.Add(mpmodel.PrincipalIdentity{
assert.NoError(t, s.PrincipalIdentityStore.Add(&mpmodel.PrincipalIdentity{
Id: &piId,
Name: &piName,
Tags: []mpmodel.Tag{{
Expand All @@ -1369,7 +1369,7 @@ func TestNSXServiceAccountService_ListNSXServiceAccountRealization(t *testing.T)
for _, ccpKey := range tt.ccpKeys {
ccpId := ccpKey
crUID := ccpKey + "-uid"
assert.NoError(t, s.ClusterControlPlaneStore.Add(model.ClusterControlPlane{
assert.NoError(t, s.ClusterControlPlaneStore.Add(&model.ClusterControlPlane{
Id: &ccpId,
Tags: []model.Tag{{
Scope: &tagScopeNSXServiceAccountCRUID,
Expand Down Expand Up @@ -1448,7 +1448,7 @@ func TestNSXServiceAccountService_GetNSXServiceAccountNameByUID(t *testing.T) {
piName := piKey.Namespace + "-" + piKey.Name
piId := piName + "-id"
crUID := piName + "-uid"
assert.NoError(t, s.PrincipalIdentityStore.Add(mpmodel.PrincipalIdentity{
assert.NoError(t, s.PrincipalIdentityStore.Add(&mpmodel.PrincipalIdentity{
Id: &piId,
Name: &piName,
Tags: []mpmodel.Tag{{
Expand All @@ -1466,7 +1466,7 @@ func TestNSXServiceAccountService_GetNSXServiceAccountNameByUID(t *testing.T) {
for _, ccpKey := range tt.ccpKeys {
ccpId := ccpKey.Namespace + "-" + ccpKey.Name
crUID := ccpId + "-uid"
assert.NoError(t, s.ClusterControlPlaneStore.Add(model.ClusterControlPlane{
assert.NoError(t, s.ClusterControlPlaneStore.Add(&model.ClusterControlPlane{
Id: &ccpId,
Tags: []model.Tag{{
Scope: &tagScopeNamespace,
Expand Down
61 changes: 32 additions & 29 deletions pkg/nsx/services/nsxserviceaccount/store.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
package nsxserviceaccount

import (
"errors"
"fmt"
"reflect"

mpmodel "github.com/vmware/vsphere-automation-sdk-go/services/nsxt-mp/nsx/model"
"github.com/vmware/vsphere-automation-sdk-go/services/nsxt/model"
Expand All @@ -18,14 +19,15 @@ type PrincipalIdentityStore struct {
}

func (s *PrincipalIdentityStore) Apply(i interface{}) error {
pis := i.(*[]mpmodel.PrincipalIdentity)
for _, pi := range *pis {
// MP resource doesn't have MarkedForDelete tag.
err := s.Add(pi)
log.V(1).Info("add PI to store", "pi", pi)
if err != nil {
return err
}
if i == nil {
return nil
}
pi := i.(*mpmodel.PrincipalIdentity)
// MP resource doesn't have MarkedForDelete tag.
err := s.Add(pi)
log.V(1).Info("add PI to store", "pi", pi)
if err != nil {
return err
}
return nil
}
Expand All @@ -40,20 +42,21 @@ type ClusterControlPlaneStore struct {
}

func (s *ClusterControlPlaneStore) Apply(i interface{}) error {
pis := i.(*[]model.ClusterControlPlane)
for _, pi := range *pis {
if pi.MarkedForDelete != nil && *pi.MarkedForDelete {
err := s.Delete(pi)
log.V(1).Info("delete PI from store", "pi", pi)
if err != nil {
return err
}
} else {
err := s.Add(pi)
log.V(1).Info("add PI to store", "pi", pi)
if err != nil {
return err
}
if i == nil {
return nil
}
ccp := i.(*model.ClusterControlPlane)
if ccp.MarkedForDelete != nil && *ccp.MarkedForDelete {
err := s.Delete(ccp)
log.V(1).Info("delete ClusterCP from store", "ClusterCP", ccp)
if err != nil {
return err
}
} else {
err := s.Add(ccp)
log.V(1).Info("add ClusterCP to store", "ClusterCP", ccp)
if err != nil {
return err
}
}
return nil
Expand All @@ -62,12 +65,12 @@ func (s *ClusterControlPlaneStore) Apply(i interface{}) error {
// keyFunc returns the key of the object.
func keyFunc(obj interface{}) (string, error) {
switch v := obj.(type) {
case model.ClusterControlPlane:
case *model.ClusterControlPlane:
return *v.Id, nil
case mpmodel.PrincipalIdentity:
case *mpmodel.PrincipalIdentity:
return *v.Name, nil
default:
return "", errors.New("keyFunc doesn't support unknown type")
return "", fmt.Errorf("keyFunc doesn't support unknown type %v", reflect.TypeOf(obj))
}
}

Expand All @@ -76,12 +79,12 @@ func keyFunc(obj interface{}) (string, error) {
func indexFunc(obj interface{}) ([]string, error) {
res := make([]string, 0, 5)
switch o := obj.(type) {
case model.ClusterControlPlane:
case *model.ClusterControlPlane:
return filterTag(o.Tags), nil
case mpmodel.PrincipalIdentity:
case *mpmodel.PrincipalIdentity:
return filterTag(common.ConvertMPTagsToTags(o.Tags)), nil
default:
return res, errors.New("indexFunc doesn't support unknown type")
return res, fmt.Errorf("indexFunc doesn't support unknown type %v", reflect.TypeOf(obj))
}
}

Expand Down
11 changes: 7 additions & 4 deletions pkg/nsx/services/nsxserviceaccount/store_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ func Test_indexFunc(t *testing.T) {
want []string
wantErr bool
}{
{"1", args{obj: ccp}, []string{"11111"}, false},
{"2", args{obj: pi}, []string{"11111"}, false},
{"1", args{obj: &ccp}, []string{"11111"}, false},
{"2", args{obj: &pi}, []string{"11111"}, false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand All @@ -49,6 +49,7 @@ func Test_keyFunc(t *testing.T) {
ccp := model.ClusterControlPlane{Id: &Id}
pi := mpmodel.PrincipalIdentity{Name: &Id}
o := model.UserInfo{}
var o2 *model.UserInfo
type args struct {
obj interface{}
}
Expand All @@ -58,9 +59,11 @@ func Test_keyFunc(t *testing.T) {
want string
wantErr bool
}{
{"1", args{obj: ccp}, Id, false},
{"2", args{obj: pi}, Id, false},
{"1", args{obj: &ccp}, Id, false},
{"2", args{obj: &pi}, Id, false},
{"0", args{obj: o}, "", true},
{"typednil", args{obj: o2}, "", true},
{"nil", args{obj: nil}, "", true},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down

0 comments on commit 56e0a6f

Please sign in to comment.