From 004f38cd02766a8751234e27448612da62d0cd8d Mon Sep 17 00:00:00 2001 From: Anubhuti Manohar Date: Fri, 8 Jun 2018 12:26:43 -0700 Subject: [PATCH] Merge fields of AAD profile during update (#3234) --- pkg/api/agentPoolOnlyApi/v20180331/merge.go | 14 +- .../agentPoolOnlyApi/v20180331/merge_test.go | 237 ++++++++++++++++++ 2 files changed, 248 insertions(+), 3 deletions(-) diff --git a/pkg/api/agentPoolOnlyApi/v20180331/merge.go b/pkg/api/agentPoolOnlyApi/v20180331/merge.go index e72b9610bc..71cf6477d8 100644 --- a/pkg/api/agentPoolOnlyApi/v20180331/merge.go +++ b/pkg/api/agentPoolOnlyApi/v20180331/merge.go @@ -2,6 +2,8 @@ package v20180331 import ( "fmt" + + "github.com/imdario/mergo" ) // Merge existing ManagedCluster attribute into mc @@ -37,9 +39,15 @@ func (mc *ManagedCluster) Merge(emc *ManagedCluster) error { // For update scenario, the default behavior is to use existing behavior mc.Properties.NetworkProfile = emc.Properties.NetworkProfile } - if mc.Properties.AADProfile == nil { - // For update scenario, the default behavior is to use existing behavior - mc.Properties.AADProfile = emc.Properties.AADProfile + + if emc.Properties.AADProfile != nil { + if mc.Properties.AADProfile == nil { + mc.Properties.AADProfile = &AADProfile{} + } + if err := mergo.Merge(mc.Properties.AADProfile, + *emc.Properties.AADProfile); err != nil { + return err + } } return nil } diff --git a/pkg/api/agentPoolOnlyApi/v20180331/merge_test.go b/pkg/api/agentPoolOnlyApi/v20180331/merge_test.go index 986f8b5902..c81b9a7e05 100644 --- a/pkg/api/agentPoolOnlyApi/v20180331/merge_test.go +++ b/pkg/api/agentPoolOnlyApi/v20180331/merge_test.go @@ -163,3 +163,240 @@ func TestMerge_EnableRBAC(t *testing.T) { } } + +func TestMerge_AAD(t *testing.T) { + // Partial AAD profile was passed during update + newMC := &ManagedCluster{ + Properties: &Properties{ + EnableRBAC: helpers.PointerToBool(true), + AADProfile: &AADProfile{ + ClientAppID: "1234-5", + ServerAppID: "1a34-5", + TenantID: "c234-5", + }, + }, + } + + existingMC := &ManagedCluster{ + Properties: &Properties{ + DNSPrefix: "something", + EnableRBAC: helpers.PointerToBool(true), + AADProfile: &AADProfile{ + ClientAppID: "1234-5", + ServerAppID: "1a34-5", + ServerAppSecret: "ba34-5", + TenantID: "c234-5", + }, + }, + } + + e := newMC.Merge(existingMC) + if e != nil { + t.Error("expect error to be nil") + } + + if newMC.Properties.AADProfile == nil { + t.Error("AADProfile should not be nil") + } + + if newMC.Properties.AADProfile.ServerAppSecret == "" { + t.Error("ServerAppSecret did not have the expected value after merge") + } + + if newMC.Properties.AADProfile.ServerAppID != "1a34-5" { + t.Error("ServerAppID did not have the expected value after merge") + } + + if newMC.Properties.AADProfile.ServerAppSecret != "ba34-5" { + t.Error("ServerAppSecret did not have the expected value after merge") + } + + if newMC.Properties.AADProfile.ClientAppID != "1234-5" { + t.Error("ClientAppID did not have the expected value after merge") + } + + if newMC.Properties.AADProfile.TenantID != "c234-5" { + t.Error("TenantID did not have the expected value after merge") + } + + // Nil AAD profile was passed during update but DM had AAD Profile + newMC = &ManagedCluster{ + Properties: &Properties{ + EnableRBAC: helpers.PointerToBool(true), + AADProfile: nil, + }, + } + + existingMC = &ManagedCluster{ + Properties: &Properties{ + DNSPrefix: "something", + EnableRBAC: helpers.PointerToBool(true), + AADProfile: &AADProfile{ + ClientAppID: "1234-5", + ServerAppID: "1a34-5", + ServerAppSecret: "ba34-5", + TenantID: "c234-5", + }, + }, + } + + e = newMC.Merge(existingMC) + if e != nil { + t.Error("expect error to be nil") + } + + if newMC.Properties.AADProfile == nil { + t.Error("AADProfile should not be nil") + } + + if newMC.Properties.AADProfile.ServerAppSecret == "" { + t.Error("ServerAppSecret did not have the expected value after merge") + } + + if newMC.Properties.AADProfile.ServerAppID != "1a34-5" { + t.Error("ServerAppID did not have the expected value after merge") + } + + if newMC.Properties.AADProfile.ServerAppSecret != "ba34-5" { + t.Error("ServerAppSecret did not have the expected value after merge") + } + + if newMC.Properties.AADProfile.ClientAppID != "1234-5" { + t.Error("ClientAppID did not have the expected value after merge") + } + + if newMC.Properties.AADProfile.TenantID != "c234-5" { + t.Error("TenantID did not have the expected value after merge") + } + + // No AAD profile set + newMC = &ManagedCluster{ + Properties: &Properties{ + EnableRBAC: helpers.PointerToBool(true), + AADProfile: nil, + }, + } + + existingMC = &ManagedCluster{ + Properties: &Properties{ + DNSPrefix: "something", + EnableRBAC: helpers.PointerToBool(true), + AADProfile: nil, + }, + } + + e = newMC.Merge(existingMC) + if e != nil { + t.Error("expect error to be nil") + } + + if newMC.Properties.AADProfile != nil { + t.Error("AADProfile should be nil") + } + + // Empty field in AAD profile was passed during update but DM had AAD Profile + newMC = &ManagedCluster{ + Properties: &Properties{ + EnableRBAC: helpers.PointerToBool(true), + AADProfile: &AADProfile{ + ClientAppID: "1234-5", + ServerAppID: "1a34-5", + ServerAppSecret: "", + TenantID: "c234-5", + }, + }, + } + + existingMC = &ManagedCluster{ + Properties: &Properties{ + DNSPrefix: "something", + EnableRBAC: helpers.PointerToBool(true), + AADProfile: &AADProfile{ + ClientAppID: "1234-5", + ServerAppID: "1a34-5", + ServerAppSecret: "ba34-5", + TenantID: "c234-5", + }, + }, + } + + e = newMC.Merge(existingMC) + if e != nil { + t.Error("expect error to be nil") + } + + if newMC.Properties.AADProfile == nil { + t.Error("AADProfile should not be nil") + } + + if newMC.Properties.AADProfile.ServerAppID != "1a34-5" { + t.Error("ServerAppID did not have the expected value after merge") + } + + if newMC.Properties.AADProfile.ServerAppSecret != "ba34-5" { + t.Error("ServerAppSecret did not have the expected value after merge") + } + + if newMC.Properties.AADProfile.ClientAppID != "1234-5" { + t.Error("ClientAppID did not have the expected value after merge") + } + + if newMC.Properties.AADProfile.TenantID != "c234-5" { + t.Error("TenantID did not have the expected value after merge") + } + + // Full AAD profile was passed during update + newMC = &ManagedCluster{ + Properties: &Properties{ + EnableRBAC: helpers.PointerToBool(true), + AADProfile: &AADProfile{ + ClientAppID: "1234-5", + ServerAppID: "1a34-5", + ServerAppSecret: "ba34-5", + TenantID: "c234-5", + }, + }, + } + + existingMC = &ManagedCluster{ + Properties: &Properties{ + DNSPrefix: "something", + EnableRBAC: helpers.PointerToBool(true), + AADProfile: &AADProfile{ + ClientAppID: "1234-5", + ServerAppID: "1a34-5", + ServerAppSecret: "ba34-5", + TenantID: "c234-5", + }, + }, + } + + e = newMC.Merge(existingMC) + if e != nil { + t.Error("expect error to be nil") + } + + if newMC.Properties.AADProfile == nil { + t.Error("AADProfile should not be nil") + } + + if newMC.Properties.AADProfile.ServerAppSecret == "" { + t.Error("ServerAppSecret did not have the expected value after merge") + } + + if newMC.Properties.AADProfile.ServerAppID != "1a34-5" { + t.Error("ServerAppID did not have the expected value after merge") + } + + if newMC.Properties.AADProfile.ServerAppSecret != "ba34-5" { + t.Error("ServerAppSecret did not have the expected value after merge") + } + + if newMC.Properties.AADProfile.ClientAppID != "1234-5" { + t.Error("ClientAppID did not have the expected value after merge") + } + + if newMC.Properties.AADProfile.TenantID != "c234-5" { + t.Error("TenantID did not have the expected value after merge") + } +}