diff --git a/pkg/api/v1alpha1/cluster_map_types.go b/pkg/api/v1alpha1/cluster_map_types.go index 990d99db4..d70f66f2f 100644 --- a/pkg/api/v1alpha1/cluster_map_types.go +++ b/pkg/api/v1alpha1/cluster_map_types.go @@ -65,6 +65,7 @@ type KubeconfigSourceFilesystem struct { // KubeconfigSourceClusterAPI get kubeconfig from clusterAPI parent cluster type KubeconfigSourceClusterAPI struct { NamespacedName `json:"clusterNamespacedName,omitempty"` + Timeout string `json:"timeout,omitempty"` } // KubeconfigSourceBundle get kubeconfig from bundle diff --git a/pkg/cluster/command.go b/pkg/cluster/command.go index b3e152da9..773aa7173 100755 --- a/pkg/cluster/command.go +++ b/pkg/cluster/command.go @@ -18,8 +18,6 @@ import ( "fmt" "io" - airshipv1 "opendev.org/airship/airshipctl/pkg/api/v1alpha1" - "opendev.org/airship/airshipctl/pkg/clusterctl/client" "opendev.org/airship/airshipctl/pkg/config" "opendev.org/airship/airshipctl/pkg/k8s/kubeconfig" "opendev.org/airship/airshipctl/pkg/log" @@ -78,11 +76,6 @@ func (cmd *GetKubeconfigCommand) RunE(cfgFactory config.Factory, writer io.Write return err } - client, err := client.NewClient(helper.TargetPath(), log.DebugEnabled(), airshipv1.DefaultClusterctl()) - if err != nil { - return err - } - var siteWide bool if cmd.ClusterName == "" { siteWide = true @@ -90,7 +83,6 @@ func (cmd *GetKubeconfigCommand) RunE(cfgFactory config.Factory, writer io.Write kubeconf := kubeconfig.NewBuilder(). WithBundle(helper.PhaseConfigBundle()). - WithClusterctlClient(client). WithClusterMap(cMap). WithClusterName(cmd.ClusterName). WithTempRoot(helper.WorkDir()). diff --git a/pkg/clusterctl/client/client.go b/pkg/clusterctl/client/client.go index ed937db59..29d3edf32 100644 --- a/pkg/clusterctl/client/client.go +++ b/pkg/clusterctl/client/client.go @@ -44,7 +44,6 @@ const ( type Interface interface { Init(kubeconfigPath, kubeconfigContext string) error Move(fromKubeconfigPath, fromKubeconfigContext, toKubeconfigPath, toKubeconfigContext, namespace string) error - GetKubeconfig(options *GetKubeconfigOptions) (string, error) Render(options RenderOptions) ([]byte, error) } @@ -65,11 +64,8 @@ type RenderOptions struct { // GetKubeconfigOptions carries all the options to retrieve kubeconfig from parent cluster type GetKubeconfigOptions struct { - // Path to parent kubeconfig file - ParentKubeconfigPath string - // Specify context within the kubeconfig file. If empty, cluster client - // will use the current context. - ParentKubeconfigContext string + // Timeout is the maximum length of time to retrieve kubeconfig + Timeout string // Namespace is the namespace in which secret is placed. ManagedClusterNamespace string // ManagedClusterName is the name of the managed cluster. @@ -168,15 +164,3 @@ func (c *Client) Render(renderOptions RenderOptions) ([]byte, error) { } return components.Yaml() } - -// GetKubeconfig is a wrapper for related cluster-api function -func (c *Client) GetKubeconfig(options *GetKubeconfigOptions) (string, error) { - return c.clusterctlClient.GetKubeconfig(clusterctlclient.GetKubeconfigOptions{ - Kubeconfig: clusterctlclient.Kubeconfig{ - Path: options.ParentKubeconfigPath, - Context: options.ParentKubeconfigContext, - }, - Namespace: options.ManagedClusterNamespace, - WorkloadClusterName: options.ManagedClusterName, - }) -} diff --git a/pkg/k8s/kubeconfig/builder.go b/pkg/k8s/kubeconfig/builder.go index 4001577ff..d47e6e46c 100644 --- a/pkg/k8s/kubeconfig/builder.go +++ b/pkg/k8s/kubeconfig/builder.go @@ -17,6 +17,7 @@ package kubeconfig import ( "fmt" + corev1 "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/client-go/tools/clientcmd" "k8s.io/client-go/tools/clientcmd/api" @@ -25,12 +26,10 @@ import ( "opendev.org/airship/airshipctl/pkg/clusterctl/client" "opendev.org/airship/airshipctl/pkg/document" "opendev.org/airship/airshipctl/pkg/fs" + "opendev.org/airship/airshipctl/pkg/k8s/utils" "opendev.org/airship/airshipctl/pkg/log" ) -// KubeconfigDefaultFileName is a default name for kubeconfig -const KubeconfigDefaultFileName = "kubeconfig" - // NewBuilder returns instance of kubeconfig builder. func NewBuilder() *Builder { return &Builder{ @@ -45,11 +44,11 @@ type Builder struct { clusterName string root string - bundle document.Bundle - clusterMap clustermap.ClusterMap - clusterctlClient client.Interface - fs fs.FileSystem - siteKubeconf *api.Config + bundle document.Bundle + client corev1.CoreV1Interface + clusterMap clustermap.ClusterMap + fs fs.FileSystem + siteKubeconf *api.Config } // WithBundle allows to set document.Bundle object that should contain kubeconfig api object @@ -76,19 +75,18 @@ func (b *Builder) WithTempRoot(root string) *Builder { return b } -// WithClusterctlClient this is used if u want to inject your own clusterctl -// mostly needed for tests -func (b *Builder) WithClusterctlClient(c client.Interface) *Builder { - b.clusterctlClient = c - return b -} - // WithFilesystem allows to set filesystem func (b *Builder) WithFilesystem(fs fs.FileSystem) *Builder { b.fs = fs return b } +// WithCoreV1Client allows to set core v1 client, use for unit tests only +func (b *Builder) WithCoreV1Client(c corev1.CoreV1Interface) *Builder { + b.client = c + return b +} + // SiteWide allows to build kubeconfig for the entire site. // If set to true ClusterName will be ignored, since all clusters are requested. func (b *Builder) SiteWide(t bool) *Builder { @@ -229,18 +227,18 @@ func (b *Builder) fromClusterAPI(clusterName string, ref v1alpha1.KubeconfigSour } defer cleanup() - if b.clusterctlClient == nil { - b.clusterctlClient, err = client.NewClient("", log.DebugEnabled(), v1alpha1.DefaultClusterctl()) + if b.client == nil { + clientSet, err := utils.FactoryFromKubeConfig(f, parentContext, utils.SetTimeout("30s")).KubernetesClientSet() if err != nil { return nil, err } + b.client = clientSet.CoreV1() } log.Debugf("Getting child kubeconfig from parent, parent context '%s', parent kubeconfig '%s'", parentContext, f) - return FromSecret(b.clusterctlClient, &client.GetKubeconfigOptions{ - ParentKubeconfigPath: f, - ParentKubeconfigContext: parentContext, + return FromSecret(b.client, &client.GetKubeconfigOptions{ + Timeout: ref.Timeout, ManagedClusterNamespace: ref.Namespace, ManagedClusterName: ref.Name, })() @@ -257,7 +255,7 @@ func (b *Builder) alreadyBuilt(clusterContext string) (bool, *api.Config) { // resulting and existing context names must be the same, otherwise error will be returned clusterKubeconfig, err := extractContext(clusterContext, clusterContext, kubeconfBytes) if err != nil { - log.Debugf("Received error when extacting context, ignoring kubeconfig. Error: %v", err) + log.Debugf("Received error when extracting context, ignoring kubeconfig. Error: %v", err) return false, nil } diff --git a/pkg/k8s/kubeconfig/builder_test.go b/pkg/k8s/kubeconfig/builder_test.go index c950c27d4..35f8ade6d 100644 --- a/pkg/k8s/kubeconfig/builder_test.go +++ b/pkg/k8s/kubeconfig/builder_test.go @@ -22,15 +22,16 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" + apiv1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + corev1 "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/client-go/tools/clientcmd" "opendev.org/airship/airshipctl/pkg/api/v1alpha1" "opendev.org/airship/airshipctl/pkg/cluster/clustermap" - "opendev.org/airship/airshipctl/pkg/clusterctl/client" "opendev.org/airship/airshipctl/pkg/document" "opendev.org/airship/airshipctl/pkg/fs" "opendev.org/airship/airshipctl/pkg/k8s/kubeconfig" - "opendev.org/airship/airshipctl/testutil/clusterctl" testfs "opendev.org/airship/airshipctl/testutil/fs" ) @@ -99,7 +100,7 @@ func TestBuilderClusterctl(t *testing.T) { expectedContexts, expectedClusters, expectedAuthInfos []string clusterMap clustermap.ClusterMap - clusterctlClient client.Interface + client corev1.CoreV1Interface fs fs.FileSystem }{ { @@ -219,23 +220,18 @@ func TestBuilderClusterctl(t *testing.T) { }, nil }, }, - clusterctlClient: func() client.Interface { - c := &clusterctl.MockInterface{ + client: func() MockCoreV1Interface { + ms := &SecretMockInterface{ Mock: mock.Mock{}, } - c.On("GetKubeconfig", &client.GetKubeconfigOptions{ - ParentKubeconfigPath: kubeconfigPath, - ParentKubeconfigContext: parentClusterID, - ManagedClusterNamespace: clustermap.DefaultClusterAPIObjNamespace, - ManagedClusterName: childClusterID, - }).Once().Return(testKubeconfigString, nil) - c.On("GetKubeconfig", &client.GetKubeconfigOptions{ - ParentKubeconfigPath: kubeconfigPath, - ParentKubeconfigContext: parentParentClusterID, - ManagedClusterNamespace: clustermap.DefaultClusterAPIObjNamespace, - ManagedClusterName: parentClusterID, - }).Once().Return(testKubeconfigStringSecond, nil) - return c + ms.On("Get", parentClusterID+"-kubeconfig", metav1.GetOptions{}). + Once().Return(&apiv1.Secret{Data: map[string][]byte{"value": []byte(testKubeconfigString)}}, nil) + ms.On("Get", childClusterID+"-kubeconfig", metav1.GetOptions{}). + Once().Return(&apiv1.Secret{Data: map[string][]byte{"value": []byte(testKubeconfigStringSecond)}}, nil) + mc := MockCoreV1Interface{MockSecrets: func(s string) corev1.SecretInterface { + return ms + }} + return mc }(), }, { @@ -267,7 +263,7 @@ func TestBuilderClusterctl(t *testing.T) { WithClusterName(tt.requestedClusterName). WithBundle(testBundle). WithTempRoot(tt.tempRoot). - WithClusterctlClient(tt.clusterctlClient). + WithCoreV1Client(tt.client). WithFilesystem(tt.fs). SiteWide(tt.siteWide). Build() diff --git a/pkg/k8s/kubeconfig/errors.go b/pkg/k8s/kubeconfig/errors.go index b37909037..81e4a8123 100644 --- a/pkg/k8s/kubeconfig/errors.go +++ b/pkg/k8s/kubeconfig/errors.go @@ -48,3 +48,20 @@ type ErrUnknownKubeconfigSourceType struct { func (e *ErrUnknownKubeconfigSourceType) Error() string { return fmt.Sprintf("unknown source type %s", e.Type) } + +// ErrClusterNameEmpty returned when cluster name is not provided +type ErrClusterNameEmpty struct { +} + +func (e ErrClusterNameEmpty) Error() string { + return "cluster name is not defined" +} + +// ErrMalformedKubeconfig error returned if kubeconfig is empty +type ErrMalformedKubeconfig struct { + ClusterName string +} + +func (e ErrMalformedKubeconfig) Error() string { + return fmt.Sprintf("retrieved kubeconfig for cluster '%s' is empty", e.ClusterName) +} diff --git a/pkg/k8s/kubeconfig/kubeconfig.go b/pkg/k8s/kubeconfig/kubeconfig.go index 41e973142..14dddf00e 100644 --- a/pkg/k8s/kubeconfig/kubeconfig.go +++ b/pkg/k8s/kubeconfig/kubeconfig.go @@ -15,9 +15,13 @@ package kubeconfig import ( + "fmt" "io" - "log" + "time" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/wait" + corev1 "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/client-go/tools/clientcmd" "k8s.io/client-go/tools/clientcmd/api" "sigs.k8s.io/yaml" @@ -26,12 +30,14 @@ import ( "opendev.org/airship/airshipctl/pkg/clusterctl/client" "opendev.org/airship/airshipctl/pkg/document" "opendev.org/airship/airshipctl/pkg/fs" + "opendev.org/airship/airshipctl/pkg/log" "opendev.org/airship/airshipctl/pkg/util" ) const ( - // KubeconfigPrefix is a prefix that is added when writing temporary kubeconfig files - KubeconfigPrefix = "kubeconfig-" + // Prefix is a prefix that is added when writing temporary kubeconfig files + Prefix = "kubeconfig-" + defaultTimeout = 30 * time.Second ) // Interface provides a uniform way to interact with kubeconfig file @@ -100,13 +106,39 @@ func FromAPIalphaV1(apiObj *v1alpha1.KubeConfig) KubeSourceFunc { } // FromSecret returns KubeSource type, uses client interface to kubernetes cluster -func FromSecret(c client.Interface, o *client.GetKubeconfigOptions) KubeSourceFunc { +func FromSecret(c corev1.CoreV1Interface, o *client.GetKubeconfigOptions) KubeSourceFunc { return func() ([]byte, error) { - data, err := c.GetKubeconfig(o) - if err != nil { + if o.ManagedClusterName == "" { + return nil, ErrClusterNameEmpty{} + } + if o.ManagedClusterNamespace == "" { + o.ManagedClusterNamespace = "default" + } + + data, exist, secretName := new([]byte), new(bool), fmt.Sprintf("%s-kubeconfig", o.ManagedClusterName) + fn := func() (bool, error) { + secret, err := c.Secrets(o.ManagedClusterNamespace).Get(secretName, metav1.GetOptions{}) + if err != nil { + log.Printf("get kubeconfig from secret failed, retrying, reason: %v", err) + return false, nil + } + + if *data, *exist = secret.Data["value"]; *exist && len(*data) > 0 { + return true, nil + } + return true, ErrMalformedKubeconfig{ClusterName: o.ManagedClusterName} + } + + duration, err := time.ParseDuration(o.Timeout) + if err != nil || duration == 0 { + duration = defaultTimeout + } + + if err = wait.PollImmediate(time.Second, duration, fn); err != nil { return nil, err } - return []byte(data), nil + + return *data, nil } } @@ -197,7 +229,7 @@ func (k *kubeConfig) WriteTempFile(root string) (string, Cleanup, error) { if err != nil { return "", nil, err } - file, err := k.fileSystem.TempFile(root, KubeconfigPrefix) + file, err := k.fileSystem.TempFile(root, Prefix) if err != nil { log.Printf("Failed to write temporary file, error %v", err) return "", nil, err diff --git a/pkg/k8s/kubeconfig/kubeconfig_test.go b/pkg/k8s/kubeconfig/kubeconfig_test.go index 541b61968..e269b2fb9 100644 --- a/pkg/k8s/kubeconfig/kubeconfig_test.go +++ b/pkg/k8s/kubeconfig/kubeconfig_test.go @@ -23,7 +23,14 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" + apiv1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/apimachinery/pkg/watch" + corev1 "k8s.io/client-go/kubernetes/typed/core/v1" v1 "k8s.io/client-go/tools/clientcmd/api/v1" kustfs "sigs.k8s.io/kustomize/api/filesys" @@ -112,46 +119,121 @@ func TestKubeconfigContent(t *testing.T) { assert.Equal(t, expectedData, actualData) } -type MockClientInterface struct { - MockGetKubeconfig func(options *client.GetKubeconfigOptions) (string, error) - client.Interface +type MockCoreV1Interface struct { + MockSecrets func(string) corev1.SecretInterface + corev1.CoreV1Interface } -func (c MockClientInterface) GetKubeconfig(o *client.GetKubeconfigOptions) (string, error) { - return c.MockGetKubeconfig(o) +func (c MockCoreV1Interface) Secrets(n string) corev1.SecretInterface { + return c.MockSecrets(n) +} + +var _ corev1.SecretInterface = &SecretMockInterface{} + +type SecretMockInterface struct { + mock.Mock +} + +func (s *SecretMockInterface) Create(_ *apiv1.Secret) (*apiv1.Secret, error) { + panic("implement me") +} + +func (s *SecretMockInterface) Update(_ *apiv1.Secret) (*apiv1.Secret, error) { + panic("implement me") +} + +func (s *SecretMockInterface) Delete(_ string, _ *metav1.DeleteOptions) error { + panic("implement me") +} + +func (s *SecretMockInterface) DeleteCollection(_ *metav1.DeleteOptions, _ metav1.ListOptions) error { + panic("implement me") +} + +func (s *SecretMockInterface) Get(name string, options metav1.GetOptions) (*apiv1.Secret, error) { + args := s.Called(name, options) + expectedResult, ok := args.Get(0).(*apiv1.Secret) + if !ok { + return nil, fmt.Errorf("wrong input") + } + return expectedResult, args.Error(1) +} + +func (s *SecretMockInterface) List(_ metav1.ListOptions) (*apiv1.SecretList, error) { + panic("implement me") +} + +func (s *SecretMockInterface) Watch(_ metav1.ListOptions) (watch.Interface, error) { + panic("implement me") +} + +func (s *SecretMockInterface) Patch(_ string, _ types.PatchType, _ []byte, _ ...string) (*apiv1.Secret, error) { + panic("implement me") } func TestFromSecret(t *testing.T) { tests := []struct { name string - expectedData string - err error + options *client.GetKubeconfigOptions + getSecret *apiv1.Secret + getErr error + expectedData []byte + expectedErr error }{ { - name: "valid kubeconfig", - expectedData: testValidKubeconfig, - err: nil, + name: "empty cluster name", + options: &client.GetKubeconfigOptions{}, + expectedErr: kubeconfig.ErrClusterNameEmpty{}, }, { - name: "failed to get kubeconfig", - expectedData: "", - err: errors.New("error"), + name: "multiple retries and error", + options: &client.GetKubeconfigOptions{ManagedClusterName: "cluster", Timeout: "1s"}, + getErr: errors.New("error"), + expectedErr: wait.ErrWaitTimeout, + }, + { + name: "empty secret object", + options: &client.GetKubeconfigOptions{ManagedClusterName: "cluster"}, + getSecret: &apiv1.Secret{}, + expectedErr: kubeconfig.ErrMalformedKubeconfig{ClusterName: "cluster"}, + }, + { + name: "empty data value", + options: &client.GetKubeconfigOptions{ManagedClusterName: "cluster"}, + getSecret: &apiv1.Secret{Data: map[string][]byte{"value": {}}}, + expectedErr: kubeconfig.ErrMalformedKubeconfig{ClusterName: "cluster"}, + }, + { + name: "successfully get kubeconfig", + options: &client.GetKubeconfigOptions{ManagedClusterName: "cluster"}, + getSecret: &apiv1.Secret{Data: map[string][]byte{"value": []byte(testValidKubeconfig)}}, + expectedData: []byte(testValidKubeconfig), }, } for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { - cl := MockClientInterface{ - MockGetKubeconfig: func(_ *client.GetKubeconfigOptions) (string, error) { return tt.expectedData, tt.err }, + sm := &SecretMockInterface{ + Mock: mock.Mock{}, } - kubeconf, err := kubeconfig.FromSecret(cl, nil)() - if tt.err != nil { + sm.On("Get", tt.options.ManagedClusterName+"-kubeconfig", metav1.GetOptions{}). + Return(tt.getSecret, tt.getErr) + + coreV1Interface := MockCoreV1Interface{ + MockSecrets: func(s string) corev1.SecretInterface { + return sm + }, + } + + kubeconf, err := kubeconfig.FromSecret(coreV1Interface, tt.options)() + if tt.expectedErr != nil { require.Error(t, err) - assert.Nil(t, kubeconf) + require.Equal(t, tt.expectedErr, err) + require.Nil(t, kubeconf) } else { require.NoError(t, err) - assert.Equal(t, []byte(tt.expectedData), kubeconf) + require.Equal(t, tt.expectedData, kubeconf) } }) } @@ -423,10 +505,10 @@ type fakeReaderWriter struct { var _ io.Reader = fakeReaderWriter{} var _ io.Writer = fakeReaderWriter{} -func (f fakeReaderWriter) Read(p []byte) (n int, err error) { +func (f fakeReaderWriter) Read(_ []byte) (n int, err error) { return 0, f.readErr } -func (f fakeReaderWriter) Write(p []byte) (n int, err error) { +func (f fakeReaderWriter) Write(_ []byte) (n int, err error) { return 0, f.writeErr } diff --git a/pkg/phase/client.go b/pkg/phase/client.go index 33fe4e1f7..2bfef7197 100644 --- a/pkg/phase/client.go +++ b/pkg/phase/client.go @@ -24,7 +24,6 @@ import ( "sigs.k8s.io/yaml" "opendev.org/airship/airshipctl/pkg/api/v1alpha1" - cctlclient "opendev.org/airship/airshipctl/pkg/clusterctl/client" "opendev.org/airship/airshipctl/pkg/container" "opendev.org/airship/airshipctl/pkg/document" "opendev.org/airship/airshipctl/pkg/events" @@ -102,19 +101,10 @@ func (p *phase) executor(docFactory document.DocFactoryFunc, return nil, err } - cctlClient, err := cctlclient.NewClient( - p.helper.PhaseBundleRoot(), - log.DebugEnabled(), - v1alpha1.DefaultClusterctl()) - if err != nil { - return nil, err - } - kubeconf := kubeconfig.NewBuilder(). WithBundle(p.helper.PhaseConfigBundle()). WithClusterMap(cMap). WithTempRoot(p.helper.WorkDir()). - WithClusterctlClient(cctlClient). WithClusterName(p.apiObj.ClusterName). SiteWide(p.apiObj.Config.SiteWideKubeconfig). Build() @@ -311,7 +301,7 @@ func (p *plan) Run(ro ifc.RunOptions) error { } // Status returns the status of phases in a given plan -func (p *plan) Status(options ifc.StatusOptions) (ifc.PlanStatus, error) { +func (p *plan) Status(_ ifc.StatusOptions) (ifc.PlanStatus, error) { for _, step := range p.apiObj.Phases { phase, err := p.phaseClient.PhaseByID(ifc.ID{Name: step.Name}) if err != nil {