diff --git a/api/v1alpha1/conditions_const.go b/api/v1alpha1/conditions_const.go index c65895015..946907d31 100644 --- a/api/v1alpha1/conditions_const.go +++ b/api/v1alpha1/conditions_const.go @@ -78,17 +78,17 @@ const ( ) const ( - // GitAPIAvailableCondition is used when Git API is available. - GitAPIAvailableCondition clusterv1.ConditionType = "GitAPIAvailable" + // AssetsClientAPIAvailableCondition is used when AssetsClient API is available. + AssetsClientAPIAvailableCondition clusterv1.ConditionType = "AssetsClientAPIAvailable" - // GitTokenOrEnvVariableNotSetReason is used when user don't specify the token or environment variable. - GitTokenOrEnvVariableNotSetReason = "GitTokenOrEnvVariableNotSet" //#nosec + // FailedCreateAssetsClientReason is used when user don't specify the token or environment variable required for initializing the assets client. + FailedCreateAssetsClientReason = "FailedCreateAssetsClient" //#nosec ) const ( - // GitReleasesSyncedCondition is used when Git releases have been synced successfully. - GitReleasesSyncedCondition clusterv1.ConditionType = "GitReleasesSynced" + // ReleasesSyncedCondition is used when releases have been synced successfully. + ReleasesSyncedCondition clusterv1.ConditionType = "ReleasesSynced" - // FailedToSyncReason is used when Git releases could not be synced. + // FailedToSyncReason is used when releases could not be synced. FailedToSyncReason = "FailedToSync" ) diff --git a/cmd/main.go b/cmd/main.go index 4b2374f34..cf1ce6527 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -27,9 +27,10 @@ import ( //+kubebuilder:scaffold:imports csov1alpha1 "github.com/SovereignCloudStack/cluster-stack-operator/api/v1alpha1" "github.com/SovereignCloudStack/cluster-stack-operator/internal/controller" + "github.com/SovereignCloudStack/cluster-stack-operator/pkg/assetsclient" + "github.com/SovereignCloudStack/cluster-stack-operator/pkg/assetsclient/fake" + "github.com/SovereignCloudStack/cluster-stack-operator/pkg/assetsclient/github" "github.com/SovereignCloudStack/cluster-stack-operator/pkg/csoversion" - githubclient "github.com/SovereignCloudStack/cluster-stack-operator/pkg/github/client" - "github.com/SovereignCloudStack/cluster-stack-operator/pkg/github/client/fake" "github.com/SovereignCloudStack/cluster-stack-operator/pkg/kube" "github.com/SovereignCloudStack/cluster-stack-operator/pkg/utillog" "github.com/SovereignCloudStack/cluster-stack-operator/pkg/workloadcluster" @@ -127,11 +128,11 @@ func main() { // Setup the context that's going to be used in controllers and for the manager. ctx := ctrl.SetupSignalHandler() - var gitFactory githubclient.Factory + var assetsClientFactory assetsclient.Factory if localMode { - gitFactory = fake.NewFactory() + assetsClientFactory = fake.NewFactory() } else { - gitFactory = githubclient.NewFactory() + assetsClientFactory = github.NewFactory() } restConfig := mgr.GetConfig() @@ -149,7 +150,7 @@ func main() { if err = (&controller.ClusterStackReconciler{ Client: mgr.GetClient(), ReleaseDirectory: releaseDir, - GitHubClientFactory: gitFactory, + AssetsClientFactory: assetsClientFactory, WatchFilterValue: watchFilterValue, }).SetupWithManager(ctx, mgr, controllerruntimecontroller.Options{MaxConcurrentReconciles: clusterStackConcurrency}); err != nil { setupLog.Error(err, "unable to create controller", "controller", "ClusterStack") @@ -162,7 +163,7 @@ func main() { ReleaseDirectory: releaseDir, WatchFilterValue: watchFilterValue, KubeClientFactory: kube.NewFactory(), - GitHubClientFactory: gitFactory, + AssetsClientFactory: assetsClientFactory, }).SetupWithManager(ctx, mgr, controllerruntimecontroller.Options{MaxConcurrentReconciles: clusterStackReleaseConcurrency}); err != nil { setupLog.Error(err, "unable to create controller", "controller", "ClusterStackRelease") os.Exit(1) @@ -175,6 +176,7 @@ func main() { WatchFilterValue: watchFilterValue, KubeClientFactory: kube.NewFactory(), WorkloadClusterFactory: workloadcluster.NewFactory(), + AssetsClientFactory: assetsClientFactory, }).SetupWithManager(ctx, mgr, controllerruntimecontroller.Options{MaxConcurrentReconciles: clusterAddonConcurrency}); err != nil { setupLog.Error(err, "unable to create controller", "controller", "ClusterAddon") os.Exit(1) diff --git a/internal/controller/clusteraddon_controller.go b/internal/controller/clusteraddon_controller.go index afafbb712..93efb191d 100644 --- a/internal/controller/clusteraddon_controller.go +++ b/internal/controller/clusteraddon_controller.go @@ -27,6 +27,7 @@ import ( "time" csov1alpha1 "github.com/SovereignCloudStack/cluster-stack-operator/api/v1alpha1" + "github.com/SovereignCloudStack/cluster-stack-operator/pkg/assetsclient" "github.com/SovereignCloudStack/cluster-stack-operator/pkg/kube" "github.com/SovereignCloudStack/cluster-stack-operator/pkg/release" "github.com/SovereignCloudStack/cluster-stack-operator/pkg/workloadcluster" @@ -67,6 +68,7 @@ type ClusterAddonReconciler struct { RestConfigSettings ReleaseDirectory string KubeClientFactory kube.Factory + AssetsClientFactory assetsclient.Factory WatchFilterValue string WorkloadClusterFactory workloadcluster.Factory } diff --git a/internal/controller/clusterstack_controller.go b/internal/controller/clusterstack_controller.go index 8e01967d1..3e43914a3 100644 --- a/internal/controller/clusterstack_controller.go +++ b/internal/controller/clusterstack_controller.go @@ -23,11 +23,12 @@ import ( "reflect" "sort" "strings" + "time" csov1alpha1 "github.com/SovereignCloudStack/cluster-stack-operator/api/v1alpha1" "github.com/SovereignCloudStack/cluster-stack-operator/internal/clusterstackrelease" + "github.com/SovereignCloudStack/cluster-stack-operator/pkg/assetsclient" "github.com/SovereignCloudStack/cluster-stack-operator/pkg/clusterstack" - githubclient "github.com/SovereignCloudStack/cluster-stack-operator/pkg/github/client" "github.com/SovereignCloudStack/cluster-stack-operator/pkg/version" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -55,7 +56,7 @@ import ( // ClusterStackReconciler reconciles a ClusterStack object. type ClusterStackReconciler struct { client.Client - GitHubClientFactory githubclient.Factory + AssetsClientFactory assetsclient.Factory ReleaseDirectory string WatchFilterValue string } @@ -112,24 +113,31 @@ func (r *ClusterStackReconciler) Reconcile(ctx context.Context, req reconcile.Re var latest *string if clusterStack.Spec.AutoSubscribe { - gc, err := r.GitHubClientFactory.NewClient(ctx) + gc, err := r.AssetsClientFactory.NewClient(ctx) if err != nil { + isSet := conditions.IsFalse(clusterStack, csov1alpha1.AssetsClientAPIAvailableCondition) conditions.MarkFalse(clusterStack, - csov1alpha1.GitAPIAvailableCondition, - csov1alpha1.GitTokenOrEnvVariableNotSetReason, + csov1alpha1.AssetsClientAPIAvailableCondition, + csov1alpha1.FailedCreateAssetsClientReason, clusterv1.ConditionSeverityError, err.Error(), ) - record.Warnf(clusterStack, "GitTokenOrEnvVariableNotSet", err.Error()) - return reconcile.Result{}, fmt.Errorf("failed to create Github client: %w", err) + record.Warnf(clusterStack, "FailedCreateAssetsClient", err.Error()) + + // give the assets client a second change + if isSet { + return reconcile.Result{RequeueAfter: 5 * time.Second}, nil + } + return reconcile.Result{}, nil } - conditions.MarkTrue(clusterStack, csov1alpha1.GitAPIAvailableCondition) + conditions.MarkTrue(clusterStack, csov1alpha1.AssetsClientAPIAvailableCondition) + latest, err = getLatestReleaseFromRemoteRepository(ctx, clusterStack, gc) if err != nil { // only log error and mark condition as false, but continue conditions.MarkFalse(clusterStack, - csov1alpha1.GitReleasesSyncedCondition, + csov1alpha1.ReleasesSyncedCondition, csov1alpha1.FailedToSyncReason, clusterv1.ConditionSeverityWarning, err.Error(), @@ -137,7 +145,7 @@ func (r *ClusterStackReconciler) Reconcile(ctx context.Context, req reconcile.Re logger.Error(err, "failed to get latest release from remote repository") } - conditions.MarkTrue(clusterStack, csov1alpha1.GitReleasesSyncedCondition) + conditions.MarkTrue(clusterStack, csov1alpha1.ReleasesSyncedCondition) } inUse, err := r.getClusterStackReleasesInUse(ctx, req.Namespace) @@ -536,21 +544,18 @@ func getLatestReadyClusterStackRelease(clusterStackReleases []*csov1alpha1.Clust return latest, k8sversion, nil } -func getLatestReleaseFromRemoteRepository(ctx context.Context, clusterStack *csov1alpha1.ClusterStack, gc githubclient.Client) (*string, error) { - ghReleases, resp, err := gc.ListRelease(ctx) +func getLatestReleaseFromRemoteRepository(ctx context.Context, clusterStack *csov1alpha1.ClusterStack, ac assetsclient.Client) (*string, error) { + releases, err := ac.ListRelease(ctx) if err != nil { - return nil, fmt.Errorf("failed to list releases on remote Git repository: %w", err) - } - if resp != nil && resp.StatusCode != 200 { - return nil, fmt.Errorf("got unexpected status from call to remote Git repository: %s", resp.Status) + return nil, fmt.Errorf("failed to list releases on remote repository: %w", err) } var clusterStacks clusterstack.ClusterStacks - for _, ghRelease := range ghReleases { - clusterStackObject, matches, err := matchesSpec(ghRelease.GetTagName(), &clusterStack.Spec) + for _, release := range releases { + clusterStackObject, matches, err := matchesSpec(release, &clusterStack.Spec) if err != nil { - return nil, fmt.Errorf("failed to get match release tag %q with spec of ClusterStack: %w", ghRelease.GetTagName(), err) + return nil, fmt.Errorf("failed to get match release tag %q with spec of ClusterStack: %w", release, err) } if matches { diff --git a/internal/controller/clusterstackrelease_controller.go b/internal/controller/clusterstackrelease_controller.go index 4ec16eb74..a62dc91a5 100644 --- a/internal/controller/clusterstackrelease_controller.go +++ b/internal/controller/clusterstackrelease_controller.go @@ -20,7 +20,6 @@ import ( "bytes" "context" "fmt" - "net/http" "os" "os/exec" "path/filepath" @@ -29,8 +28,8 @@ import ( "time" csov1alpha1 "github.com/SovereignCloudStack/cluster-stack-operator/api/v1alpha1" + "github.com/SovereignCloudStack/cluster-stack-operator/pkg/assetsclient" "github.com/SovereignCloudStack/cluster-stack-operator/pkg/clusterstack" - githubclient "github.com/SovereignCloudStack/cluster-stack-operator/pkg/github/client" "github.com/SovereignCloudStack/cluster-stack-operator/pkg/kube" "github.com/SovereignCloudStack/cluster-stack-operator/pkg/release" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -57,7 +56,7 @@ type ClusterStackReleaseReconciler struct { RESTConfig *rest.Config ReleaseDirectory string KubeClientFactory kube.Factory - GitHubClientFactory githubclient.Factory + AssetsClientFactory assetsclient.Factory externalTracker external.ObjectTracker clusterStackRelDownloadDirectoryMutex sync.Mutex WatchFilterValue string @@ -116,27 +115,32 @@ func (r *ClusterStackReleaseReconciler) Reconcile(ctx context.Context, req recon if download { conditions.MarkFalse(clusterStackRelease, csov1alpha1.ClusterStackReleaseAssetsReadyCondition, csov1alpha1.ReleaseAssetsNotDownloadedYetReason, clusterv1.ConditionSeverityInfo, "assets not downloaded yet") - // this is the point where we download the release from github + // this is the point where we download the release // acquire lock so that only one reconcile loop can download the release r.clusterStackRelDownloadDirectoryMutex.Lock() - defer r.clusterStackRelDownloadDirectoryMutex.Unlock() - gc, err := r.GitHubClientFactory.NewClient(ctx) + ac, err := r.AssetsClientFactory.NewClient(ctx) if err != nil { + isSet := conditions.IsFalse(clusterStackRelease, csov1alpha1.AssetsClientAPIAvailableCondition) conditions.MarkFalse(clusterStackRelease, - csov1alpha1.GitAPIAvailableCondition, - csov1alpha1.GitTokenOrEnvVariableNotSetReason, + csov1alpha1.AssetsClientAPIAvailableCondition, + csov1alpha1.FailedCreateAssetsClientReason, clusterv1.ConditionSeverityError, err.Error(), ) - record.Warnf(clusterStackRelease, "GitTokenOrEnvVariableNotSet", err.Error()) - return reconcile.Result{}, fmt.Errorf("failed to create Github client: %w", err) + record.Warnf(clusterStackRelease, "FailedCreateAssetsClient", err.Error()) + + // give the assets client a second change + if isSet { + return reconcile.Result{RequeueAfter: 5 * time.Second}, nil + } + return reconcile.Result{}, nil } - conditions.MarkTrue(clusterStackRelease, csov1alpha1.GitAPIAvailableCondition) + conditions.MarkTrue(clusterStackRelease, csov1alpha1.AssetsClientAPIAvailableCondition) - if err := downloadReleaseAssets(ctx, releaseTag, releaseAssets.LocalDownloadPath, gc); err != nil { + if err := downloadReleaseAssets(ctx, releaseTag, releaseAssets.LocalDownloadPath, ac); err != nil { return reconcile.Result{}, fmt.Errorf("failed to download release assets: %w", err) } @@ -215,18 +219,8 @@ func (r *ClusterStackReleaseReconciler) reconcileDelete(ctx context.Context, rel return reconcile.Result{}, nil } -func downloadReleaseAssets(ctx context.Context, releaseTag, downloadPath string, gc githubclient.Client) error { - repoRelease, resp, err := gc.GetReleaseByTag(ctx, releaseTag) - if err != nil { - return fmt.Errorf("failed to fetch release tag %q: %w", releaseTag, err) - } - if resp.StatusCode != http.StatusOK { - return fmt.Errorf("failed to fetch release tag %s with status code %d", releaseTag, resp.StatusCode) - } - - assetlist := []string{"metadata.yaml", "cluster-addon-values.yaml", "cluster-addon", "cluster-class"} - - if err := gc.DownloadReleaseAssets(ctx, repoRelease, downloadPath, assetlist); err != nil { +func downloadReleaseAssets(ctx context.Context, releaseTag, downloadPath string, ac assetsclient.Client) error { + if err := ac.DownloadReleaseAssets(ctx, releaseTag, downloadPath); err != nil { // if download failed for some reason, delete the release directory so that it can be retried in the next reconciliation if err := os.RemoveAll(downloadPath); err != nil { return fmt.Errorf("failed to remove release: %w", err) diff --git a/internal/controller/controller_suite_test.go b/internal/controller/controller_suite_test.go index ccd8340dd..ad2383302 100644 --- a/internal/controller/controller_suite_test.go +++ b/internal/controller/controller_suite_test.go @@ -53,14 +53,14 @@ var _ = BeforeSuite(func() { Expect((&ClusterStackReconciler{ Client: testEnv.Manager.GetClient(), ReleaseDirectory: "./../../test/releases", - GitHubClientFactory: testEnv.GitHubClientFactory, + AssetsClientFactory: testEnv.AssetsClientFactory, }).SetupWithManager(ctx, testEnv.Manager, c.Options{})).To(Succeed()) Expect((&ClusterStackReleaseReconciler{ Client: testEnv.Manager.GetClient(), RESTConfig: testEnv.Manager.GetConfig(), KubeClientFactory: kube.NewFactory(), - GitHubClientFactory: testEnv.GitHubClientFactory, + AssetsClientFactory: testEnv.AssetsClientFactory, ReleaseDirectory: "./../../test/releases", }).SetupWithManager(ctx, testEnv.Manager, c.Options{})).To(Succeed()) diff --git a/internal/test/helpers/envtest.go b/internal/test/helpers/envtest.go index 046aefef4..d41c8cccd 100644 --- a/internal/test/helpers/envtest.go +++ b/internal/test/helpers/envtest.go @@ -31,8 +31,8 @@ import ( csov1alpha1 "github.com/SovereignCloudStack/cluster-stack-operator/api/v1alpha1" "github.com/SovereignCloudStack/cluster-stack-operator/internal/test/helpers/builder" - githubclient "github.com/SovereignCloudStack/cluster-stack-operator/pkg/github/client" - githubmocks "github.com/SovereignCloudStack/cluster-stack-operator/pkg/github/client/mocks" + "github.com/SovereignCloudStack/cluster-stack-operator/pkg/assetsclient" + assetsclientmocks "github.com/SovereignCloudStack/cluster-stack-operator/pkg/assetsclient/mocks" "github.com/SovereignCloudStack/cluster-stack-operator/pkg/test/utils" g "github.com/onsi/ginkgo/v2" corev1 "k8s.io/api/core/v1" @@ -145,8 +145,8 @@ type ( cancel context.CancelFunc kind *kind.Provider WorkloadClusterClient *kubernetes.Clientset - GitHubClientFactory githubclient.Factory - GitHubClient *githubmocks.Client + AssetsClientFactory assetsclient.Factory + AssetsClient *assetsclientmocks.Client } ) @@ -183,14 +183,14 @@ func NewTestEnvironment() *TestEnvironment { klog.Fatalf("unable to create manager pod namespace: %s", err) } - githubClient := &githubmocks.Client{} + assetsClient := &assetsclientmocks.Client{} testEnv := &TestEnvironment{ Manager: mgr, Client: mgr.GetClient(), Config: mgr.GetConfig(), - GitHubClientFactory: githubmocks.NewGitHubFactory(githubClient), - GitHubClient: githubClient, + AssetsClientFactory: assetsclientmocks.NewAssetsClientFactory(assetsClient), + AssetsClient: assetsClient, } if ifCreateKind() { diff --git a/internal/test/integration/github/integration_suite_test.go b/internal/test/integration/github/integration_suite_test.go index 3c594fb6c..4531ca3fe 100644 --- a/internal/test/integration/github/integration_suite_test.go +++ b/internal/test/integration/github/integration_suite_test.go @@ -20,7 +20,7 @@ import ( "github.com/SovereignCloudStack/cluster-stack-operator/internal/controller" "github.com/SovereignCloudStack/cluster-stack-operator/internal/test/helpers" - githubclient "github.com/SovereignCloudStack/cluster-stack-operator/pkg/github/client" + githubclient "github.com/SovereignCloudStack/cluster-stack-operator/pkg/assetsclient/github" "github.com/SovereignCloudStack/cluster-stack-operator/pkg/kube" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -47,14 +47,14 @@ var _ = BeforeSuite(func() { testEnv = helpers.NewTestEnvironment() Expect((&controller.ClusterStackReconciler{ Client: testEnv.Manager.GetClient(), - GitHubClientFactory: githubclient.NewFactory(), + AssetsClientFactory: githubclient.NewFactory(), ReleaseDirectory: "/tmp/downloads", }).SetupWithManager(ctx, testEnv.Manager, controllerruntimecontroller.Options{})).To(Succeed()) Expect((&controller.ClusterStackReleaseReconciler{ Client: testEnv.Manager.GetClient(), RESTConfig: testEnv.Manager.GetConfig(), KubeClientFactory: kube.NewFactory(), - GitHubClientFactory: githubclient.NewFactory(), + AssetsClientFactory: githubclient.NewFactory(), ReleaseDirectory: "/tmp/downloads", }).SetupWithManager(ctx, testEnv.Manager, controllerruntimecontroller.Options{})).To(Succeed()) diff --git a/internal/test/integration/github/integration_test.go b/internal/test/integration/github/integration_test.go index 2efe783b3..c9625bf17 100644 --- a/internal/test/integration/github/integration_test.go +++ b/internal/test/integration/github/integration_test.go @@ -69,7 +69,22 @@ var _ = Describe("ClusterStackReconciler", func() { }) AfterEach(func() { - Expect(testEnv.Cleanup(ctx, testNs, clusterStack)).To(Succeed()) + Eventually(func() error { + return testEnv.Cleanup(ctx, testNs, clusterStack) + }, timeout, interval).Should(BeNil()) + }) + + It("checks if the AssetsClientAPIAvailableCondition condition is true", func() { + Eventually(func() bool { + var foundClusterStackRelease csov1alpha1.ClusterStackRelease + if err := testEnv.Get(ctx, clusterStackReleaseKey, &foundClusterStackRelease); err != nil { + testEnv.GetLogger().Error(err, "failed to get clusterStackRelease", "key", clusterStackReleaseKey) + return false + } + + testEnv.GetLogger().Info("status condition of cluster stack release", "key", clusterStackReleaseKey, "status condition", foundClusterStackRelease.Status.Conditions) + return utils.IsPresentAndTrue(ctx, testEnv.Client, clusterStackReleaseKey, &foundClusterStackRelease, csov1alpha1.AssetsClientAPIAvailableCondition) + }, timeout, interval).Should(BeTrue()) }) It("creates the cluster stack release object", func() { diff --git a/internal/test/integration/workloadcluster/controller_suite_test.go b/internal/test/integration/workloadcluster/controller_suite_test.go index f548744b2..44b36e542 100644 --- a/internal/test/integration/workloadcluster/controller_suite_test.go +++ b/internal/test/integration/workloadcluster/controller_suite_test.go @@ -55,14 +55,14 @@ var _ = BeforeSuite(func() { Expect((&controller.ClusterStackReconciler{ Client: testEnv.Manager.GetClient(), ReleaseDirectory: "./../../../../test/releases", - GitHubClientFactory: testEnv.GitHubClientFactory, + AssetsClientFactory: testEnv.AssetsClientFactory, }).SetupWithManager(ctx, testEnv.Manager, controllerruntimecontroller.Options{})).To(Succeed()) Expect((&controller.ClusterStackReleaseReconciler{ Client: testEnv.Manager.GetClient(), RESTConfig: testEnv.Manager.GetConfig(), KubeClientFactory: kube.NewFactory(), - GitHubClientFactory: testEnv.GitHubClientFactory, + AssetsClientFactory: testEnv.AssetsClientFactory, ReleaseDirectory: "./../../../../test/releases", }).SetupWithManager(ctx, testEnv.Manager, controllerruntimecontroller.Options{})).To(Succeed()) @@ -75,6 +75,7 @@ var _ = BeforeSuite(func() { ReleaseDirectory: "./../../../../test/releases", KubeClientFactory: kube.NewFactory(), WorkloadClusterFactory: workloadcluster.NewFactory(), + AssetsClientFactory: testEnv.AssetsClientFactory, }).SetupWithManager(ctx, testEnv.Manager, controllerruntimecontroller.Options{})).To(Succeed()) go func() { diff --git a/pkg/assetsclient/client.go b/pkg/assetsclient/client.go new file mode 100644 index 000000000..1d545837f --- /dev/null +++ b/pkg/assetsclient/client.go @@ -0,0 +1,33 @@ +/* +Copyright 2023 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package assetsclient contains interface for talking to assets repositories. +package assetsclient + +import ( + "context" +) + +// Client contains functions to talk to list and download assets. +type Client interface { + DownloadReleaseAssets(ctx context.Context, tag, path string) error + ListRelease(ctx context.Context) ([]string, error) +} + +// Factory is a factory to generate assets clients. +type Factory interface { + NewClient(ctx context.Context) (Client, error) +} diff --git a/pkg/assetsclient/fake/client.go b/pkg/assetsclient/fake/client.go new file mode 100644 index 000000000..953ab8544 --- /dev/null +++ b/pkg/assetsclient/fake/client.go @@ -0,0 +1,60 @@ +/* +Copyright 2023 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package fake defines a fake Gitub client. +package fake + +import ( + "context" + + "github.com/SovereignCloudStack/cluster-stack-operator/pkg/assetsclient" + "github.com/go-logr/logr" + "sigs.k8s.io/controller-runtime/pkg/log" +) + +type fakeClient struct { + log logr.Logger +} + +type factory struct{} + +var _ = assetsclient.Client(&fakeClient{}) + +var _ = assetsclient.Factory(&factory{}) + +// NewFactory returns a new factory for assets clients. +func NewFactory() assetsclient.Factory { + return &factory{} +} + +func (*factory) NewClient(ctx context.Context) (assetsclient.Client, error) { + logger := log.FromContext(ctx) + + return &fakeClient{ + log: logger, + }, nil +} + +func (c *fakeClient) ListRelease(_ context.Context) ([]string, error) { + c.log.Info("WARNING: called ListRelease of fake assets client") + return nil, nil +} + +// DownloadReleaseAssets downloads a list of release assets. +func (c *fakeClient) DownloadReleaseAssets(_ context.Context, _, _ string) error { + c.log.Info("WARNING: called DownloadReleaseAssets of fake assets client") + return nil +} diff --git a/pkg/github/client/github_client.go b/pkg/assetsclient/github/client.go similarity index 76% rename from pkg/github/client/github_client.go rename to pkg/assetsclient/github/client.go index 83e4a114a..4eb4118d7 100644 --- a/pkg/github/client/github_client.go +++ b/pkg/assetsclient/github/client.go @@ -1,5 +1,5 @@ /* -Copyright 2023 The Kubernetes Authors. +Copyright 2024 The Kubernetes Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package client +package github import ( "context" @@ -23,24 +23,12 @@ import ( "net/http" "os" "path/filepath" - "strings" + "github.com/SovereignCloudStack/cluster-stack-operator/pkg/assetsclient" "github.com/google/go-github/v52/github" "golang.org/x/oauth2" ) -// Client contains all functions to talk to Github API. -type Client interface { - DownloadReleaseAssets(ctx context.Context, release *github.RepositoryRelease, path string, assetlist []string) error - GetReleaseByTag(ctx context.Context, tag string) (*github.RepositoryRelease, *github.Response, error) - ListRelease(ctx context.Context) ([]*github.RepositoryRelease, *github.Response, error) -} - -// Factory is a factory to generate Github clients. -type Factory interface { - NewClient(ctx context.Context) (Client, error) -} - type realGhClient struct { client *github.Client httpclient *http.Client @@ -50,18 +38,18 @@ type realGhClient struct { type factory struct{} -var _ = Client(&realGhClient{}) +var _ = assetsclient.Client(&realGhClient{}) -var _ = Factory(&factory{}) +var _ = assetsclient.Factory(&factory{}) // NewFactory returns a new factory for Github clients. -func NewFactory() Factory { +func NewFactory() assetsclient.Factory { return &factory{} } -var _ = Client(&realGhClient{}) +var _ = assetsclient.Client(&realGhClient{}) -func (*factory) NewClient(ctx context.Context) (Client, error) { +func (*factory) NewClient(ctx context.Context) (assetsclient.Client, error) { creds, err := NewGitConfig() if err != nil { return nil, fmt.Errorf("failed to create git config: %w", err) @@ -83,16 +71,26 @@ func (*factory) NewClient(ctx context.Context) (Client, error) { }, nil } -func (c *realGhClient) ListRelease(ctx context.Context) ([]*github.RepositoryRelease, *github.Response, error) { +func (c *realGhClient) ListRelease(ctx context.Context) ([]string, error) { repoRelease, response, err := c.client.Repositories.ListReleases(ctx, c.orgName, c.repoName, &github.ListOptions{}) if err != nil { - return nil, nil, fmt.Errorf("failed to list releases: %w", err) + return nil, fmt.Errorf("failed to list releases: %w", err) } - return repoRelease, response, nil + if response != nil && response.StatusCode != 200 { + return nil, fmt.Errorf("got unexpected status from call to remote repository: %s", response.Status) + } + + releases := []string{} + + for _, release := range repoRelease { + releases = append(releases, *release.Name) + } + + return releases, nil } -func (c *realGhClient) GetReleaseByTag(ctx context.Context, tag string) (*github.RepositoryRelease, *github.Response, error) { +func (c *realGhClient) getReleaseByTag(ctx context.Context, tag string) (*github.RepositoryRelease, *github.Response, error) { repoRelease, response, err := c.client.Repositories.GetReleaseByTag(ctx, c.orgName, c.repoName, tag) if err != nil { return nil, nil, fmt.Errorf("failed to get release tag: %w", err) @@ -102,15 +100,21 @@ func (c *realGhClient) GetReleaseByTag(ctx context.Context, tag string) (*github } // DownloadReleaseAssets downloads a list of release assets. -func (c *realGhClient) DownloadReleaseAssets(ctx context.Context, release *github.RepositoryRelease, path string, assetlist []string) error { +func (c *realGhClient) DownloadReleaseAssets(ctx context.Context, tag, path string) error { + release, response, err := c.getReleaseByTag(ctx, tag) + if err != nil { + return fmt.Errorf("failed to fetch release tag %s: %w", tag, err) + } + + if response.StatusCode != http.StatusOK { + return fmt.Errorf("failed to fetch release tag %s with status code %d: %w", tag, response.StatusCode, err) + } + if err := os.MkdirAll(path, os.ModePerm); err != nil { //nolint:gosec //nolint:ignore return fmt.Errorf("failed to create destination directory: %w", err) } // Extract the release assets for _, asset := range release.Assets { - if !contains(assetlist, asset.GetName()) { - continue - } assetPath := filepath.Join(path, asset.GetName()) // Create a temporary file (inside the dest dir) to save the downloaded asset file assetFile, err := os.Create(filepath.Clean(assetPath)) @@ -145,7 +149,7 @@ func (c *realGhClient) DownloadReleaseAssets(ctx context.Context, release *githu return nil } -func (c *realGhClient) handleRedirect(ctx context.Context, url string, assetFile *os.File) error { +func (c *realGhClient) handleRedirect(ctx context.Context, url string, assetFile *os.File) (reterr error) { req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, http.NoBody) if err != nil { return fmt.Errorf("failed to define http get request: %w", err) @@ -156,6 +160,13 @@ func (c *realGhClient) handleRedirect(ctx context.Context, url string, assetFile return fmt.Errorf("failed to get URL %q: %w", url, err) } + defer func() { + err := resp.Body.Close() + if reterr == nil { + reterr = err + } + }() + if resp.StatusCode != http.StatusOK { return fmt.Errorf("failed to download asset, HTTP status code: %d", resp.StatusCode) } @@ -164,9 +175,6 @@ func (c *realGhClient) handleRedirect(ctx context.Context, url string, assetFile return fmt.Errorf("failed to copy http response in file: %w", err) } - if err := resp.Body.Close(); err != nil { - return fmt.Errorf("failed to close body of response: %w", err) - } return nil } @@ -196,12 +204,3 @@ func verifyAccess(ctx context.Context, client *github.Client, creds GitConfig) e } return nil } - -func contains(source []string, ghAsset string) bool { - for _, a := range source { - if a == ghAsset || strings.Contains(ghAsset, a) { - return true - } - } - return false -} diff --git a/pkg/github/client/credentials.go b/pkg/assetsclient/github/credentials.go similarity index 96% rename from pkg/github/client/credentials.go rename to pkg/assetsclient/github/credentials.go index 542f4e421..6845bccb8 100644 --- a/pkg/github/client/credentials.go +++ b/pkg/assetsclient/github/credentials.go @@ -14,8 +14,8 @@ See the License for the specific language governing permissions and limitations under the License. */ -// Package client contains interface for github client. -package client +// Package github provides utilities for talking to github API. +package github import ( "fmt" diff --git a/pkg/assetsclient/mocks/Client.go b/pkg/assetsclient/mocks/Client.go new file mode 100644 index 000000000..227e2c674 --- /dev/null +++ b/pkg/assetsclient/mocks/Client.go @@ -0,0 +1,68 @@ +// Code generated by mockery v2.32.4. DO NOT EDIT. + +package mocks + +import ( + context "context" + + mock "github.com/stretchr/testify/mock" +) + +// Client is an autogenerated mock type for the Client type +type Client struct { + mock.Mock +} + +// DownloadReleaseAssets provides a mock function with given fields: ctx, tag, path +func (_m *Client) DownloadReleaseAssets(ctx context.Context, tag string, path string) error { + ret := _m.Called(ctx, tag, path) + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, string, string) error); ok { + r0 = rf(ctx, tag, path) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// ListRelease provides a mock function with given fields: ctx +func (_m *Client) ListRelease(ctx context.Context) ([]string, error) { + ret := _m.Called(ctx) + + var r0 []string + var r1 error + if rf, ok := ret.Get(0).(func(context.Context) ([]string, error)); ok { + return rf(ctx) + } + if rf, ok := ret.Get(0).(func(context.Context) []string); ok { + r0 = rf(ctx) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]string) + } + } + + if rf, ok := ret.Get(1).(func(context.Context) error); ok { + r1 = rf(ctx) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// NewClient creates a new instance of Client. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewClient(t interface { + mock.TestingT + Cleanup(func()) +}) *Client { + mock := &Client{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/pkg/github/client/mocks/Factory.go b/pkg/assetsclient/mocks/Factory.go similarity index 70% rename from pkg/github/client/mocks/Factory.go rename to pkg/assetsclient/mocks/Factory.go index f7046f9fa..a57ed99f0 100644 --- a/pkg/github/client/mocks/Factory.go +++ b/pkg/assetsclient/mocks/Factory.go @@ -5,7 +5,7 @@ package mocks import ( context "context" - client "github.com/SovereignCloudStack/cluster-stack-operator/pkg/github/client" + assetsclient "github.com/SovereignCloudStack/cluster-stack-operator/pkg/assetsclient" mock "github.com/stretchr/testify/mock" ) @@ -16,19 +16,19 @@ type Factory struct { } // NewClient provides a mock function with given fields: ctx -func (_m *Factory) NewClient(ctx context.Context) (client.Client, error) { +func (_m *Factory) NewClient(ctx context.Context) (assetsclient.Client, error) { ret := _m.Called(ctx) - var r0 client.Client + var r0 assetsclient.Client var r1 error - if rf, ok := ret.Get(0).(func(context.Context) (client.Client, error)); ok { + if rf, ok := ret.Get(0).(func(context.Context) (assetsclient.Client, error)); ok { return rf(ctx) } - if rf, ok := ret.Get(0).(func(context.Context) client.Client); ok { + if rf, ok := ret.Get(0).(func(context.Context) assetsclient.Client); ok { r0 = rf(ctx) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(client.Client) + r0 = ret.Get(0).(assetsclient.Client) } } diff --git a/pkg/assetsclient/mocks/assetsFactory.go b/pkg/assetsclient/mocks/assetsFactory.go new file mode 100644 index 000000000..46fc102b1 --- /dev/null +++ b/pkg/assetsclient/mocks/assetsFactory.go @@ -0,0 +1,37 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package mocks implement important mocking interface of packer. +package mocks + +import ( + "context" + + "github.com/SovereignCloudStack/cluster-stack-operator/pkg/assetsclient" +) + +type assetsClientFactory struct { + client *Client +} + +// NewAssetsClientFactory returns a mocked assets client factory. +func NewAssetsClientFactory(client *Client) assetsclient.Factory { + return &assetsClientFactory{client: client} +} + +func (f *assetsClientFactory) NewClient(_ context.Context) (assetsclient.Client, error) { + return f.client, nil +} diff --git a/pkg/github/client/fake/github_client.go b/pkg/github/client/fake/github_client.go deleted file mode 100644 index 90e0bd098..000000000 --- a/pkg/github/client/fake/github_client.go +++ /dev/null @@ -1,69 +0,0 @@ -/* -Copyright 2023 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -// Package fake defines a fake Gitub client. -package fake - -import ( - "context" - "net/http" - - "github.com/SovereignCloudStack/cluster-stack-operator/pkg/github/client" - "github.com/go-logr/logr" - "github.com/google/go-github/v52/github" - "sigs.k8s.io/controller-runtime/pkg/log" -) - -type fakeClient struct { - log logr.Logger -} - -type factory struct{} - -var _ = client.Client(&fakeClient{}) - -var _ = client.Factory(&factory{}) - -// NewFactory returns a new factory for Github clients. -func NewFactory() client.Factory { - return &factory{} -} - -func (*factory) NewClient(ctx context.Context) (client.Client, error) { - logger := log.FromContext(ctx) - - return &fakeClient{ - log: logger, - }, nil -} - -func (c *fakeClient) ListRelease(_ context.Context) ([]*github.RepositoryRelease, *github.Response, error) { - c.log.Info("WARNING: called ListRelease of fake Github client") - resp := &github.Response{Response: &http.Response{StatusCode: http.StatusOK}} - return nil, resp, nil -} - -func (c *fakeClient) GetReleaseByTag(_ context.Context, _ string) (*github.RepositoryRelease, *github.Response, error) { - c.log.Info("WARNING: called GetReleaseByTag of fake Github client") - resp := &github.Response{Response: &http.Response{StatusCode: http.StatusOK}} - return nil, resp, nil -} - -// DownloadReleaseAssets downloads a list of release assets. -func (c *fakeClient) DownloadReleaseAssets(_ context.Context, _ *github.RepositoryRelease, _ string, _ []string) error { - c.log.Info("WARNING: called DownloadReleaseAssets of fake Github client") - return nil -} diff --git a/pkg/github/client/mocks/Client.go b/pkg/github/client/mocks/Client.go deleted file mode 100644 index 279ba3df4..000000000 --- a/pkg/github/client/mocks/Client.go +++ /dev/null @@ -1,113 +0,0 @@ -// Code generated by mockery v2.32.4. DO NOT EDIT. - -package mocks - -import ( - context "context" - - github "github.com/google/go-github/v52/github" - mock "github.com/stretchr/testify/mock" -) - -// Client is an autogenerated mock type for the Client type -type Client struct { - mock.Mock -} - -// DownloadReleaseAssets provides a mock function with given fields: ctx, release, path, assetlist -func (_m *Client) DownloadReleaseAssets(ctx context.Context, release *github.RepositoryRelease, path string, assetlist []string) error { - ret := _m.Called(ctx, release, path, assetlist) - - var r0 error - if rf, ok := ret.Get(0).(func(context.Context, *github.RepositoryRelease, string, []string) error); ok { - r0 = rf(ctx, release, path, assetlist) - } else { - r0 = ret.Error(0) - } - - return r0 -} - -// GetReleaseByTag provides a mock function with given fields: ctx, tag -func (_m *Client) GetReleaseByTag(ctx context.Context, tag string) (*github.RepositoryRelease, *github.Response, error) { - ret := _m.Called(ctx, tag) - - var r0 *github.RepositoryRelease - var r1 *github.Response - var r2 error - if rf, ok := ret.Get(0).(func(context.Context, string) (*github.RepositoryRelease, *github.Response, error)); ok { - return rf(ctx, tag) - } - if rf, ok := ret.Get(0).(func(context.Context, string) *github.RepositoryRelease); ok { - r0 = rf(ctx, tag) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*github.RepositoryRelease) - } - } - - if rf, ok := ret.Get(1).(func(context.Context, string) *github.Response); ok { - r1 = rf(ctx, tag) - } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*github.Response) - } - } - - if rf, ok := ret.Get(2).(func(context.Context, string) error); ok { - r2 = rf(ctx, tag) - } else { - r2 = ret.Error(2) - } - - return r0, r1, r2 -} - -// ListRelease provides a mock function with given fields: ctx -func (_m *Client) ListRelease(ctx context.Context) ([]*github.RepositoryRelease, *github.Response, error) { - ret := _m.Called(ctx) - - var r0 []*github.RepositoryRelease - var r1 *github.Response - var r2 error - if rf, ok := ret.Get(0).(func(context.Context) ([]*github.RepositoryRelease, *github.Response, error)); ok { - return rf(ctx) - } - if rf, ok := ret.Get(0).(func(context.Context) []*github.RepositoryRelease); ok { - r0 = rf(ctx) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).([]*github.RepositoryRelease) - } - } - - if rf, ok := ret.Get(1).(func(context.Context) *github.Response); ok { - r1 = rf(ctx) - } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*github.Response) - } - } - - if rf, ok := ret.Get(2).(func(context.Context) error); ok { - r2 = rf(ctx) - } else { - r2 = ret.Error(2) - } - - return r0, r1, r2 -} - -// NewClient creates a new instance of Client. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. -// The first argument is typically a *testing.T value. -func NewClient(t interface { - mock.TestingT - Cleanup(func()) -}) *Client { - mock := &Client{} - mock.Mock.Test(t) - - t.Cleanup(func() { mock.AssertExpectations(t) }) - - return mock -} diff --git a/pkg/github/client/mocks/gfactory.go b/pkg/github/client/mocks/gfactory.go deleted file mode 100644 index b6e0e986a..000000000 --- a/pkg/github/client/mocks/gfactory.go +++ /dev/null @@ -1,23 +0,0 @@ -// Package mocks implement important mocking interface of packer. -package mocks - -import ( - "context" - - githubclient "github.com/SovereignCloudStack/cluster-stack-operator/pkg/github/client" -) - -type githubFactory struct { - client *Client -} - -// NewGitHubFactory returns a mocked Github client. -func NewGitHubFactory(client *Client) githubclient.Factory { - return &githubFactory{client: client} -} - -var _ = githubclient.Factory(&githubFactory{}) - -func (f *githubFactory) NewClient(_ context.Context) (githubclient.Client, error) { - return f.client, nil -} diff --git a/pkg/kube/mocks/kFactory.go b/pkg/kube/mocks/kFactory.go new file mode 100644 index 000000000..67fa03a22 --- /dev/null +++ b/pkg/kube/mocks/kFactory.go @@ -0,0 +1,22 @@ +// Package mocks implement important mocking interface of kube. +package mocks + +import ( + "github.com/SovereignCloudStack/cluster-stack-operator/pkg/kube" + "k8s.io/client-go/rest" +) + +type kubeFactory struct { + client *Client +} + +// NewKubeFactory returns packer factory interface. +func NewKubeFactory(client *Client) kube.Factory { + return &kubeFactory{client: client} +} + +var _ = kube.Factory(&kubeFactory{}) + +func (f *kubeFactory) NewClient(_ string, _ *rest.Config) kube.Client { + return f.client +} diff --git a/pkg/workloadcluster/mocks/Client.go b/pkg/workloadcluster/mocks/Client.go new file mode 100644 index 000000000..481ee91dd --- /dev/null +++ b/pkg/workloadcluster/mocks/Client.go @@ -0,0 +1,55 @@ +// Code generated by mockery v2.32.4. DO NOT EDIT. + +package mocks + +import ( + context "context" + + mock "github.com/stretchr/testify/mock" + rest "k8s.io/client-go/rest" +) + +// Client is an autogenerated mock type for the Client type +type Client struct { + mock.Mock +} + +// RestConfig provides a mock function with given fields: ctx +func (_m *Client) RestConfig(ctx context.Context) (*rest.Config, error) { + ret := _m.Called(ctx) + + var r0 *rest.Config + var r1 error + if rf, ok := ret.Get(0).(func(context.Context) (*rest.Config, error)); ok { + return rf(ctx) + } + if rf, ok := ret.Get(0).(func(context.Context) *rest.Config); ok { + r0 = rf(ctx) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*rest.Config) + } + } + + if rf, ok := ret.Get(1).(func(context.Context) error); ok { + r1 = rf(ctx) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// NewClient creates a new instance of Client. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewClient(t interface { + mock.TestingT + Cleanup(func()) +}) *Client { + mock := &Client{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/pkg/workloadcluster/mocks/Factory.go b/pkg/workloadcluster/mocks/Factory.go new file mode 100644 index 000000000..bc17a009b --- /dev/null +++ b/pkg/workloadcluster/mocks/Factory.go @@ -0,0 +1,45 @@ +// Code generated by mockery v2.32.4. DO NOT EDIT. + +package mocks + +import ( + mock "github.com/stretchr/testify/mock" + client "sigs.k8s.io/controller-runtime/pkg/client" + + workloadcluster "github.com/SovereignCloudStack/cluster-stack-operator/pkg/workloadcluster" +) + +// Factory is an autogenerated mock type for the Factory type +type Factory struct { + mock.Mock +} + +// NewClient provides a mock function with given fields: name, namespace, controllerClient +func (_m *Factory) NewClient(name string, namespace string, controllerClient client.Client) workloadcluster.Client { + ret := _m.Called(name, namespace, controllerClient) + + var r0 workloadcluster.Client + if rf, ok := ret.Get(0).(func(string, string, client.Client) workloadcluster.Client); ok { + r0 = rf(name, namespace, controllerClient) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(workloadcluster.Client) + } + } + + return r0 +} + +// NewFactory creates a new instance of Factory. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewFactory(t interface { + mock.TestingT + Cleanup(func()) +}) *Factory { + mock := &Factory{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +}