Skip to content

Commit

Permalink
Fix iam auth role upgrades (#3399)
Browse files Browse the repository at this point in the history
* Adding checks for IAM MapRoles and MapUsers

* Adding logic to fetch aws iam object
  • Loading branch information
pokearu authored Sep 20, 2022
1 parent 70e3e4c commit f88b7af
Show file tree
Hide file tree
Showing 7 changed files with 152 additions and 4 deletions.
11 changes: 10 additions & 1 deletion controllers/resource/fetcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,15 @@ func (r *CapiResourceFetcher) oidcConfig(ctx context.Context, name, namespace st
return clusterOIDC, nil
}

func (r *CapiResourceFetcher) awsIamConfig(ctx context.Context, name, namespace string) (*anywherev1.AWSIamConfig, error) {
awsIamConfig := &anywherev1.AWSIamConfig{}
err := r.FetchObjectByName(ctx, name, namespace, awsIamConfig)
if err != nil {
return nil, err
}
return awsIamConfig, nil
}

func (r *CapiResourceFetcher) ControlPlane(ctx context.Context, cs *anywherev1.Cluster) (*controlplanev1.KubeadmControlPlane, error) {
// Fetch capi cluster
capiCluster := &clusterv1.Cluster{}
Expand Down Expand Up @@ -437,7 +446,7 @@ func (r *CapiResourceFetcher) OIDCConfig(ctx context.Context, ref *anywherev1.Re
}

func (r *CapiResourceFetcher) FetchAppliedSpec(ctx context.Context, cs *anywherev1.Cluster) (*cluster.Spec, error) {
return cluster.BuildSpecForCluster(ctx, cs, r.bundles, r.eksdRelease, nil, nil, r.oidcConfig)
return cluster.BuildSpecForCluster(ctx, cs, r.bundles, r.eksdRelease, nil, nil, r.oidcConfig, r.awsIamConfig)
}

func (r *CapiResourceFetcher) ExistingVSphereDatacenterConfig(ctx context.Context, cs *anywherev1.Cluster, wnc anywherev1.WorkerNodeGroupConfiguration) (*anywherev1.VSphereDatacenterConfig, error) {
Expand Down
27 changes: 25 additions & 2 deletions pkg/cluster/fetch.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,12 @@ type EksdReleaseFetch func(ctx context.Context, name, namespace string) (*eksdv1

type OIDCFetch func(ctx context.Context, name, namespace string) (*v1alpha1.OIDCConfig, error)

type AWSIamConfigFetch func(ctx context.Context, name, namespace string) (*v1alpha1.AWSIamConfig, error)

// BuildSpec constructs a cluster.Spec for an eks-a cluster by retrieving all
// necessary objects using fetch methods
// This is deprecated in favour of BuildSpec
func BuildSpecForCluster(ctx context.Context, cluster *v1alpha1.Cluster, bundlesFetch BundlesFetch, eksdReleaseFetch EksdReleaseFetch, gitOpsFetch GitOpsFetch, fluxConfigFetch FluxConfigFetch, oidcFetch OIDCFetch) (*Spec, error) {
func BuildSpecForCluster(ctx context.Context, cluster *v1alpha1.Cluster, bundlesFetch BundlesFetch, eksdReleaseFetch EksdReleaseFetch, gitOpsFetch GitOpsFetch, fluxConfigFetch FluxConfigFetch, oidcFetch OIDCFetch, awsIamConfigFetch AWSIamConfigFetch) (*Spec, error) {
bundles, err := GetBundlesForCluster(ctx, cluster, bundlesFetch)
if err != nil {
return nil, err
Expand Down Expand Up @@ -58,7 +60,11 @@ func BuildSpecForCluster(ctx context.Context, cluster *v1alpha1.Cluster, bundles
if err != nil {
return nil, err
}
return BuildSpecFromBundles(cluster, bundles, WithEksdRelease(eksd), WithGitOpsConfig(gitOpsConfig), WithFluxConfig(fluxConfig), WithOIDCConfig(oidcConfig))
awsIamConfig, err := GetAWSIamConfigForCluster(ctx, cluster, awsIamConfigFetch)
if err != nil {
return nil, err
}
return BuildSpecFromBundles(cluster, bundles, WithEksdRelease(eksd), WithGitOpsConfig(gitOpsConfig), WithFluxConfig(fluxConfig), WithOIDCConfig(oidcConfig), WithAWSIamConfig(awsIamConfig))
}

func GetBundlesForCluster(ctx context.Context, cluster *v1alpha1.Cluster, fetch BundlesFetch) (*v1alpha1release.Bundles, error) {
Expand Down Expand Up @@ -154,6 +160,23 @@ func GetOIDCForCluster(ctx context.Context, cluster *v1alpha1.Cluster, fetch OID
return nil, nil
}

func GetAWSIamConfigForCluster(ctx context.Context, cluster *v1alpha1.Cluster, fetch AWSIamConfigFetch) (*v1alpha1.AWSIamConfig, error) {
if fetch == nil || cluster.Spec.IdentityProviderRefs == nil {
return nil, nil
}

for _, identityProvider := range cluster.Spec.IdentityProviderRefs {
if identityProvider.Kind == v1alpha1.AWSIamConfigKind {
awsIamConfig, err := fetch(ctx, identityProvider.Name, cluster.Namespace)
if err != nil {
return nil, fmt.Errorf("failed fetching AWSIamConfig for cluster: %v", err)
}
return awsIamConfig, nil
}
}
return nil, nil
}

// BuildSpec constructs a cluster.Spec for an eks-a cluster by retrieving all
// necessary objects from the cluster using a kubernetes client
func BuildSpec(ctx context.Context, client Client, cluster *v1alpha1.Cluster) (*Spec, error) {
Expand Down
59 changes: 59 additions & 0 deletions pkg/cluster/fetch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,65 @@ func TestGetFluxConfigForCluster(t *testing.T) {
g.Expect(gotFlux).To(Equal(wantFlux))
}

func TestGetAWSIamConfigForCluster(t *testing.T) {
g := NewWithT(t)
c := &anywherev1.Cluster{
ObjectMeta: metav1.ObjectMeta{
Name: "eksa-cluster",
Namespace: "eksa",
},
Spec: anywherev1.ClusterSpec{
IdentityProviderRefs: []anywherev1.Ref{
{
Kind: anywherev1.AWSIamConfigKind,
Name: "eksa-cluster",
},
},
},
}
wantIamConfig := &anywherev1.AWSIamConfig{
TypeMeta: metav1.TypeMeta{},
ObjectMeta: metav1.ObjectMeta{},
Spec: anywherev1.AWSIamConfigSpec{},
Status: anywherev1.AWSIamConfigStatus{},
}

mockFetch := func(ctx context.Context, name, namespace string) (*anywherev1.AWSIamConfig, error) {
g.Expect(name).To(Equal(c.Name))
g.Expect(namespace).To(Equal(c.Namespace))

return wantIamConfig, nil
}

gotIamConfig, err := cluster.GetAWSIamConfigForCluster(context.Background(), c, mockFetch)
g.Expect(err).To(BeNil())
g.Expect(gotIamConfig).To(Equal(wantIamConfig))
}

func TestGetAWSIamConfigForClusterIsNil(t *testing.T) {
g := NewWithT(t)
c := &anywherev1.Cluster{
ObjectMeta: metav1.ObjectMeta{
Name: "eksa-cluster",
Namespace: "eksa",
},
Spec: anywherev1.ClusterSpec{
IdentityProviderRefs: nil,
},
}
var wantIamConfig *anywherev1.AWSIamConfig
mockFetch := func(ctx context.Context, name, namespace string) (*anywherev1.AWSIamConfig, error) {
g.Expect(name).To(Equal(c.Name))
g.Expect(namespace).To(Equal(c.Namespace))

return wantIamConfig, nil
}

gotIamConfig, err := cluster.GetAWSIamConfigForCluster(context.Background(), c, mockFetch)
g.Expect(err).To(BeNil())
g.Expect(gotIamConfig).To(Equal(wantIamConfig))
}

type buildSpecTest struct {
*WithT
ctx context.Context
Expand Down
6 changes: 6 additions & 0 deletions pkg/cluster/spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,12 @@ func WithOIDCConfig(oidcConfig *eksav1alpha1.OIDCConfig) SpecOpt {
}
}

func WithAWSIamConfig(awsIamConfig *eksav1alpha1.AWSIamConfig) SpecOpt {
return func(s *Spec) {
s.AWSIamConfig = awsIamConfig
}
}

func NewSpec(opts ...SpecOpt) *Spec {
s := &Spec{
Config: &Config{},
Expand Down
18 changes: 17 additions & 1 deletion pkg/clustermanager/cluster_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
_ "embed"
"errors"
"fmt"
"reflect"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -79,6 +80,7 @@ type ClusterClient interface {
GetEksaGitOpsConfig(ctx context.Context, gitOpsConfigName string, kubeconfigFile string, namespace string) (*v1alpha1.GitOpsConfig, error)
GetEksaFluxConfig(ctx context.Context, fluxConfigName string, kubeconfigFile string, namespace string) (*v1alpha1.FluxConfig, error)
GetEksaOIDCConfig(ctx context.Context, oidcConfigName string, kubeconfigFile string, namespace string) (*v1alpha1.OIDCConfig, error)
GetEksaAWSIamConfig(ctx context.Context, awsIamConfigName string, kubeconfigFile string, namespace string) (*v1alpha1.AWSIamConfig, error)
DeleteCluster(ctx context.Context, managementCluster, clusterToDelete *types.Cluster) error
DeleteGitOpsConfig(ctx context.Context, managementCluster *types.Cluster, gitOpsName, namespace string) error
DeleteOIDCConfig(ctx context.Context, managementCluster *types.Cluster, oidcConfigName, oidcConfigNamespace string) error
Expand Down Expand Up @@ -510,6 +512,14 @@ func (c *ClusterManager) EKSAClusterSpecChanged(ctx context.Context, cluster *ty
}
}

if newClusterSpec.AWSIamConfig != nil && currentClusterSpec.AWSIamConfig != nil {
if !reflect.DeepEqual(newClusterSpec.AWSIamConfig.Spec.MapRoles, currentClusterSpec.AWSIamConfig.Spec.MapRoles) ||
!reflect.DeepEqual(newClusterSpec.AWSIamConfig.Spec.MapUsers, currentClusterSpec.AWSIamConfig.Spec.MapUsers) {
logger.V(3).Info("AWSIamConfig changes detected")
return true, nil
}
}

logger.V(3).Info("Clusters are the same")
return false, nil
}
Expand Down Expand Up @@ -1056,7 +1066,7 @@ func (c *ClusterManager) GetCurrentClusterSpec(ctx context.Context, clus *types.
}

func (c *ClusterManager) buildSpecForCluster(ctx context.Context, clus *types.Cluster, eksaCluster *v1alpha1.Cluster) (*cluster.Spec, error) {
return cluster.BuildSpecForCluster(ctx, eksaCluster, c.bundlesFetcher(clus), c.eksdReleaseFetcher(clus), c.gitOpsFetcher(clus), c.fluxConfigFetcher(clus), c.oidcFetcher(clus))
return cluster.BuildSpecForCluster(ctx, eksaCluster, c.bundlesFetcher(clus), c.eksdReleaseFetcher(clus), c.gitOpsFetcher(clus), c.fluxConfigFetcher(clus), c.oidcFetcher(clus), c.awsIamConfigFetcher(clus))
}

func (c *ClusterManager) bundlesFetcher(cluster *types.Cluster) cluster.BundlesFetch {
Expand Down Expand Up @@ -1089,6 +1099,12 @@ func (c *ClusterManager) oidcFetcher(cluster *types.Cluster) cluster.OIDCFetch {
}
}

func (c *ClusterManager) awsIamConfigFetcher(cluster *types.Cluster) cluster.AWSIamConfigFetch {
return func(ctx context.Context, name, namespace string) (*v1alpha1.AWSIamConfig, error) {
return c.clusterClient.GetEksaAWSIamConfig(ctx, name, cluster.KubeconfigFile, namespace)
}
}

func (c *ClusterManager) DeleteGitOpsConfig(ctx context.Context, managementCluster *types.Cluster, name string, namespace string) error {
return c.clusterClient.DeleteGitOpsConfig(ctx, managementCluster, name, namespace)
}
Expand Down
20 changes: 20 additions & 0 deletions pkg/clustermanager/cluster_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1839,6 +1839,26 @@ func TestClusterManagerClusterSpecChangedGitOpsDefault(t *testing.T) {
assert.False(t, diff, "No changes should have been detected")
}

func TestClusterManagerClusterSpecChangedAWSIamConfigChanged(t *testing.T) {
tt := newSpecChangedTest(t)
tt.clusterSpec.Cluster.Spec.IdentityProviderRefs = []v1alpha1.Ref{{Kind: v1alpha1.AWSIamConfigKind, Name: tt.clusterName}}
tt.clusterSpec.AWSIamConfig = &v1alpha1.AWSIamConfig{}
tt.oldClusterConfig = tt.clusterSpec.Cluster.DeepCopy()
oldIamConfig := tt.clusterSpec.AWSIamConfig.DeepCopy()
tt.clusterSpec.AWSIamConfig = &v1alpha1.AWSIamConfig{Spec: v1alpha1.AWSIamConfigSpec{
MapRoles: []v1alpha1.MapRoles{},
}}

tt.mocks.client.EXPECT().GetEksaCluster(tt.ctx, tt.cluster, tt.clusterSpec.Cluster.Name).Return(tt.oldClusterConfig, nil)
tt.mocks.client.EXPECT().GetBundles(tt.ctx, tt.cluster.KubeconfigFile, tt.cluster.Name, "").Return(test.Bundles(t), nil)
tt.mocks.client.EXPECT().GetEksdRelease(tt.ctx, gomock.Any(), constants.EksaSystemNamespace, gomock.Any())
tt.mocks.client.EXPECT().GetEksaAWSIamConfig(tt.ctx, tt.clusterSpec.Cluster.Spec.IdentityProviderRefs[0].Name, tt.cluster.KubeconfigFile, tt.clusterSpec.Cluster.Namespace).Return(oldIamConfig, nil)
diff, err := tt.clusterManager.EKSAClusterSpecChanged(tt.ctx, tt.cluster, tt.clusterSpec)

assert.Nil(t, err, "Error should be nil")
assert.True(t, diff, "Changes should have been detected")
}

type testSetup struct {
*WithT
clusterManager *clustermanager.ClusterManager
Expand Down
15 changes: 15 additions & 0 deletions pkg/clustermanager/mocks/client_and_networking.go

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

0 comments on commit f88b7af

Please sign in to comment.