Skip to content

✨ Add AWS IAM support #677

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

Merged
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
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ kubectl edit klusterlet klusterlet

### Integration tests

The integration tests are written in the [test/integration](test/integration) directory. They start a kubenretes
The integration tests are written in the [test/integration](test/integration) directory. They start a kubernetes
api server locally with [controller-runtime](https://book.kubebuilder.io/reference/envtest), and run the tests against
the local api server.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,12 @@ spec:
{{if .AppliedManifestWorkEvictionGracePeriod}}
- "--appliedmanifestwork-eviction-grace-period={{ .AppliedManifestWorkEvictionGracePeriod }}"
{{end}}
{{if .RegistrationDriver.AuthType}}
- "--registration-auth={{ .RegistrationDriver.AuthType }}"
{{end}}
{{if eq .RegistrationDriver.AuthType "awsirsa"}}
- "--hub-cluster-arn={{ .RegistrationDriver.AwsIrsa.HubClusterArn }}"
{{end}}
env:
- name: POD_NAME
valueFrom:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,12 @@ spec:
{{if gt .RegistrationKubeAPIBurst 0}}
- "--kube-api-burst={{ .RegistrationKubeAPIBurst }}"
{{end}}
{{if .RegistrationDriver.AuthType}}
- "--registration-auth={{ .RegistrationDriver.AuthType }}"
{{end}}
{{if eq .RegistrationDriver.AuthType "awsirsa"}}
- "--hub-cluster-arn={{ .RegistrationDriver.AwsIrsa.HubClusterArn }}"
{{end}}
env:
- name: POD_NAME
valueFrom:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ const (
klusterletFinalizer = "operator.open-cluster-management.io/klusterlet-cleanup"
managedResourcesEvictionTimestampAnno = "operator.open-cluster-management.io/managed-resources-eviction-timestamp"
klusterletNamespaceLabelKey = "operator.open-cluster-management.io/klusterlet"
AwsIrsaAuthType = "awsirsa"
)

type klusterletController struct {
Expand Down Expand Up @@ -111,6 +112,15 @@ func NewKlusterletController(
ToController("KlusterletController", recorder)
}

type AwsIrsa struct {
HubClusterArn string
}

type RegistrationDriver struct {
AuthType string
AwsIrsa *AwsIrsa
}

// klusterletConfig is used to render the template of hub manifests
type klusterletConfig struct {
KlusterletName string
Expand Down Expand Up @@ -174,7 +184,8 @@ type klusterletConfig struct {
DisableAddonNamespace bool

// Labels of the agents are synced from klusterlet CR.
Labels map[string]string
Labels map[string]string
RegistrationDriver RegistrationDriver
}

// If multiplehubs feature gate is enabled, using the bootstrapkubeconfigs from klusterlet CR.
Expand Down Expand Up @@ -309,7 +320,20 @@ func (n *klusterletController) sync(ctx context.Context, controllerContext facto
config.ClientCertExpirationSeconds = klusterlet.Spec.RegistrationConfiguration.ClientCertExpirationSeconds
config.RegistrationKubeAPIQPS = float32(klusterlet.Spec.RegistrationConfiguration.KubeAPIQPS)
config.RegistrationKubeAPIBurst = klusterlet.Spec.RegistrationConfiguration.KubeAPIBurst

//Configuring Registration driver depending on registration auth
if &klusterlet.Spec.RegistrationConfiguration.RegistrationDriver != nil &&
klusterlet.Spec.RegistrationConfiguration.RegistrationDriver.AuthType == AwsIrsaAuthType {
config.RegistrationDriver = RegistrationDriver{
AuthType: klusterlet.Spec.RegistrationConfiguration.RegistrationDriver.AuthType,
AwsIrsa: &AwsIrsa{
HubClusterArn: klusterlet.Spec.RegistrationConfiguration.RegistrationDriver.AwsIrsa.HubClusterArn,
},
}
} else {
config.RegistrationDriver = RegistrationDriver{
AuthType: klusterlet.Spec.RegistrationConfiguration.RegistrationDriver.AuthType,
}
}
// construct cluster annotations string, the final format is "key1=value1,key2=value2"
var annotationsArray []string
for k, v := range commonhelpers.FilterClusterAnnotations(klusterlet.Spec.RegistrationConfiguration.ClusterAnnotations) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ func newKlusterlet(name, namespace, clustername string) *operatorapiv1.Klusterle

func newKlusterletHosted(name, namespace, clustername string) *operatorapiv1.Klusterlet {
klusterlet := newKlusterlet(name, namespace, clustername)
klusterlet.Spec.RegistrationConfiguration.RegistrationDriver = operatorapiv1.RegistrationDriver{}
klusterlet.Spec.DeployOption.Mode = operatorapiv1.InstallModeHosted
klusterlet.Finalizers = append(klusterlet.Finalizers, klusterletHostedFinalizer)
return klusterlet
Expand Down Expand Up @@ -374,7 +375,46 @@ func getDeployments(actions []clienttesting.Action, verb, suffix string) *appsv1
return nil
}

func assertRegistrationDeployment(t *testing.T, actions []clienttesting.Action, verb, serverURL, clusterName string, replica int32) {
func assertKlusterletDeployment(t *testing.T, actions []clienttesting.Action, verb, serverURL, clusterName string) {
deployment := getDeployments(actions, verb, "agent")
if deployment == nil {
t.Errorf("klusterlet deployment not found")
return
}
if len(deployment.Spec.Template.Spec.Containers) != 1 {
t.Errorf("Expect 1 containers in deployment spec, actual %d", len(deployment.Spec.Template.Spec.Containers))
return
}

args := deployment.Spec.Template.Spec.Containers[0].Args
expectedArgs := []string{
"/registration-operator",
"agent",
fmt.Sprintf("--spoke-cluster-name=%s", clusterName),
"--bootstrap-kubeconfig=/spoke/bootstrap/kubeconfig",
}

if serverURL != "" {
expectedArgs = append(expectedArgs, fmt.Sprintf("--spoke-external-server-urls=%s", serverURL))
}

expectedArgs = append(expectedArgs, "--agent-id=", "--workload-source-driver=kube", "--workload-source-config=/spoke/hub-kubeconfig/kubeconfig")

if *deployment.Spec.Replicas == 1 {
expectedArgs = append(expectedArgs, "--disable-leader-election")
}

expectedArgs = append(expectedArgs, "--status-sync-interval=60s", "--kube-api-qps=20", "--kube-api-burst=60",
"--registration-auth=awsirsa", "--hub-cluster-arn=arneks:us-west-2:123456789012:cluster/hub-cluster1")

if !equality.Semantic.DeepEqual(args, expectedArgs) {
t.Errorf("Expect args %v, but got %v", expectedArgs, args)
return
}

}

func assertRegistrationDeployment(t *testing.T, actions []clienttesting.Action, verb, serverURL, clusterName string, replica int32, awsAuth bool) {
deployment := getDeployments(actions, verb, "registration-agent")
if deployment == nil {
t.Errorf("registration deployment not found")
Expand Down Expand Up @@ -402,7 +442,9 @@ func assertRegistrationDeployment(t *testing.T, actions []clienttesting.Action,
}

expectedArgs = append(expectedArgs, "--kube-api-qps=10", "--kube-api-burst=60")

if awsAuth {
expectedArgs = append(expectedArgs, "--registration-auth=awsirsa", "--hub-cluster-arn=arneks:us-west-2:123456789012:cluster/hub-cluster1")
}
if !equality.Semantic.DeepEqual(args, expectedArgs) {
t.Errorf("Expect args %v, but got %v", expectedArgs, args)
return
Expand Down Expand Up @@ -944,6 +986,67 @@ func TestGetServersFromKlusterlet(t *testing.T) {
}
}

func TestAWSIrsaAuthInSingletonMode(t *testing.T) {
klusterlet := newKlusterlet("klusterlet", "testns", "cluster1")
awsIrsaRegistrationDriver := operatorapiv1.RegistrationDriver{
AuthType: AwsIrsaAuthType,
AwsIrsa: &operatorapiv1.AwsIrsa{
HubClusterArn: "arneks:us-west-2:123456789012:cluster/hub-cluster1",
},
}
klusterlet.Spec.RegistrationConfiguration.RegistrationDriver = awsIrsaRegistrationDriver
klusterlet.Spec.DeployOption.Mode = operatorapiv1.InstallModeSingleton
hubSecret := newSecret(helpers.HubKubeConfig, "testns")
hubSecret.Data["kubeconfig"] = []byte("dummuykubeconnfig")
hubSecret.Data["cluster-name"] = []byte("cluster1")
objects := []runtime.Object{
newNamespace("testns"),
newSecret(helpers.BootstrapHubKubeConfig, "testns"),
hubSecret,
}

syncContext := testingcommon.NewFakeSyncContext(t, "klusterlet")
controller := newTestController(t, klusterlet, syncContext.Recorder(), nil, false,
objects...)

err := controller.controller.sync(context.TODO(), syncContext)
if err != nil {
t.Errorf("Expected non error when sync, %v", err)
}

assertKlusterletDeployment(t, controller.kubeClient.Actions(), createVerb, "", "cluster1")
}

func TestAWSIrsaAuthInNonSingletonMode(t *testing.T) {
klusterlet := newKlusterlet("klusterlet", "testns", "cluster1")
awsIrsaRegistrationDriver := operatorapiv1.RegistrationDriver{
AuthType: AwsIrsaAuthType,
AwsIrsa: &operatorapiv1.AwsIrsa{
HubClusterArn: "arneks:us-west-2:123456789012:cluster/hub-cluster1",
},
}
klusterlet.Spec.RegistrationConfiguration.RegistrationDriver = awsIrsaRegistrationDriver
hubSecret := newSecret(helpers.HubKubeConfig, "testns")
hubSecret.Data["kubeconfig"] = []byte("dummuykubeconnfig")
hubSecret.Data["cluster-name"] = []byte("cluster1")
objects := []runtime.Object{
newNamespace("testns"),
newSecret(helpers.BootstrapHubKubeConfig, "testns"),
hubSecret,
}

syncContext := testingcommon.NewFakeSyncContext(t, "klusterlet")
controller := newTestController(t, klusterlet, syncContext.Recorder(), nil, false,
objects...)

err := controller.controller.sync(context.TODO(), syncContext)
if err != nil {
t.Errorf("Expected non error when sync, %v", err)
}

assertRegistrationDeployment(t, controller.kubeClient.Actions(), createVerb, "", "cluster1", 1, true)
}

func TestReplica(t *testing.T) {
klusterlet := newKlusterlet("klusterlet", "testns", "cluster1")
hubSecret := newSecret(helpers.HubKubeConfig, "testns")
Expand All @@ -965,7 +1068,7 @@ func TestReplica(t *testing.T) {
}

// should have 1 replica for registration deployment and 0 for work
assertRegistrationDeployment(t, controller.kubeClient.Actions(), createVerb, "", "cluster1", 1)
assertRegistrationDeployment(t, controller.kubeClient.Actions(), createVerb, "", "cluster1", 1, false)
assertWorkDeployment(t, controller.kubeClient.Actions(), createVerb, "cluster1", operatorapiv1.InstallModeDefault, 0)

klusterlet = newKlusterlet("klusterlet", "testns", "cluster1")
Expand Down Expand Up @@ -1010,7 +1113,7 @@ func TestReplica(t *testing.T) {
}

// should have 3 replicas for clusters with multiple nodes
assertRegistrationDeployment(t, controller.kubeClient.Actions(), "update", "", "cluster1", 3)
assertRegistrationDeployment(t, controller.kubeClient.Actions(), "update", "", "cluster1", 3, false)
assertWorkDeployment(t, controller.kubeClient.Actions(), "update", "cluster1", operatorapiv1.InstallModeDefault, 3)
}

Expand All @@ -1031,7 +1134,7 @@ func TestClusterNameChange(t *testing.T) {
}

// Check if deployment has the right cluster name set
assertRegistrationDeployment(t, controller.kubeClient.Actions(), createVerb, "", "cluster1", 1)
assertRegistrationDeployment(t, controller.kubeClient.Actions(), createVerb, "", "cluster1", 1, false)

operatorAction := controller.operatorClient.Actions()
testingcommon.AssertActions(t, operatorAction, "patch")
Expand Down Expand Up @@ -1061,7 +1164,7 @@ func TestClusterNameChange(t *testing.T) {
if err != nil {
t.Errorf("Expected non error when sync, %v", err)
}
assertRegistrationDeployment(t, controller.kubeClient.Actions(), "update", "", "", 1)
assertRegistrationDeployment(t, controller.kubeClient.Actions(), "update", "", "", 1, false)

// Update hubconfigsecret and sync again
hubSecret.Data["cluster-name"] = []byte("cluster2")
Expand Down Expand Up @@ -1099,7 +1202,7 @@ func TestClusterNameChange(t *testing.T) {
if err != nil {
t.Errorf("Expected non error when sync, %v", err)
}
assertRegistrationDeployment(t, controller.kubeClient.Actions(), "update", "https://localhost", "cluster3", 1)
assertRegistrationDeployment(t, controller.kubeClient.Actions(), "update", "https://localhost", "cluster3", 1, false)
assertWorkDeployment(t, controller.kubeClient.Actions(), "update", "cluster3", "", 0)
}

Expand Down
12 changes: 12 additions & 0 deletions pkg/registration/helpers/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package helpers
import (
"embed"
"net/url"
"regexp"

"github.com/openshift/library-go/pkg/assets"
"github.com/openshift/library-go/pkg/operator/resource/resourceapply"
Expand Down Expand Up @@ -158,3 +159,14 @@ func IsCSRSupported(nativeClient kubernetes.Interface) (bool, bool, error) {
}
return v1CSRSupported, v1beta1CSRSupported, nil
}

// IsEksArnWellFormed checks if the EKS cluster ARN is well-formed
// Example of a well-formed ARN: arn:aws:eks:us-west-2:123456789012:cluster/my-cluster
func IsEksArnWellFormed(eksArn string) bool {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add some comment for this func, it will be easier to understand what is the format for the arn.

pattern := "^arn:aws:eks:([a-zA-Z0-9-]+):(\\d{12}):cluster/([a-zA-Z0-9-]+)$"
matched, err := regexp.MatchString(pattern, eksArn)
if err != nil {
return false
}
return matched
}
4 changes: 4 additions & 0 deletions pkg/registration/helpers/testing/testinghelpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,10 @@ type TestCert struct {
Key []byte
}

// TODO: Remove this struct once we have the function fully implemented for the AWSIRSADriver
type TestIrsaRequest struct {
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might add some TODO or comment here, I am not quite sure why this is added.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are using the struct in aws_irsa_test.go on lines 38 and 64, which is setting up a test irsa driver for each test case.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, I think we could add a //TODO here so we will not forget the followup task later :)


func NewHubKubeconfigSecret(namespace, name, resourceVersion string, cert *TestCert, data map[string][]byte) *corev1.Secret {
secret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Expand Down
68 changes: 68 additions & 0 deletions pkg/registration/register/aws_irsa/aws.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
package aws_irsa

import (
"k8s.io/client-go/tools/cache"

cluster "open-cluster-management.io/api/client/cluster/clientset/versioned"
managedclusterv1client "open-cluster-management.io/api/client/cluster/clientset/versioned/typed/cluster/v1"
managedclusterinformers "open-cluster-management.io/api/client/cluster/informers/externalversions/cluster"
managedclusterv1lister "open-cluster-management.io/api/client/cluster/listers/cluster/v1"
)

type AWSIRSAControl interface {
isApproved(name string) (bool, error)
generateEKSKubeConfig(name string) ([]byte, error)

// Informer is public so we can add indexer outside
Informer() cache.SharedIndexInformer
}

var _ AWSIRSAControl = &v1AWSIRSAControl{}

type v1AWSIRSAControl struct {
hubManagedClusterInformer cache.SharedIndexInformer
hubManagedClusterLister managedclusterv1lister.ManagedClusterLister
hubManagedClusterClient managedclusterv1client.ManagedClusterInterface
}

func (v *v1AWSIRSAControl) isApproved(name string) (bool, error) {
// TODO: check if the managedclusuter cr on hub has required condition and is approved
approved := false

return approved, nil
}

func (v *v1AWSIRSAControl) generateEKSKubeConfig(name string) ([]byte, error) {
// TODO: generate and return kubeconfig
return nil, nil
}

func (v *v1AWSIRSAControl) Informer() cache.SharedIndexInformer {
return v.hubManagedClusterInformer
}

//TODO: Uncomment the below once required in the aws irsa authentication implementation
/*
func (v *v1AWSIRSAControl) get(name string) (metav1.Object, error) {
managedcluster, err := v.hubManagedClusterLister.Get(name)
switch {
case apierrors.IsNotFound(err):
// fallback to fetching managedcluster from hub apiserver in case it is not cached by informer yet
managedcluster, err = v.hubManagedClusterClient.Get(context.Background(), name, metav1.GetOptions{})
if apierrors.IsNotFound(err) {
return nil, fmt.Errorf("unable to get managedcluster %q. It might have already been deleted", name)
}
case err != nil:
return nil, err
}
return managedcluster, nil
}
*/

func NewAWSIRSAControl(hubManagedClusterInformer managedclusterinformers.Interface, hubManagedClusterClient cluster.Interface) (AWSIRSAControl, error) {
return &v1AWSIRSAControl{
hubManagedClusterInformer: hubManagedClusterInformer.V1().ManagedClusters().Informer(),
hubManagedClusterLister: hubManagedClusterInformer.V1().ManagedClusters().Lister(),
hubManagedClusterClient: hubManagedClusterClient.ClusterV1().ManagedClusters(),
}, nil
}
Loading
Loading