diff --git a/charts/fleet/templates/configmap.yaml b/charts/fleet/templates/configmap.yaml index a801cb6024..4477402111 100644 --- a/charts/fleet/templates/configmap.yaml +++ b/charts/fleet/templates/configmap.yaml @@ -22,5 +22,6 @@ data: "agentNamespace": "{{.Values.bootstrap.agentNamespace}}" }, "webhookReceiverURL": "{{.Values.webhookReceiverURL}}", - "githubURLPrefix": "{{.Values.githubURLPrefix}}" + "githubURLPrefix": "{{.Values.githubURLPrefix}}", + "gitClientTimeout": "{{.Values.gitClientTimeout}}" } diff --git a/charts/fleet/values.yaml b/charts/fleet/values.yaml index c867f20cf6..2970d458e0 100644 --- a/charts/fleet/values.yaml +++ b/charts/fleet/values.yaml @@ -37,6 +37,10 @@ ignoreClusterRegistrationLabels: false # comma separated list of domains or ip addresses that will not use the proxy noProxy: 127.0.0.0/8,10.0.0.0/8,172.16.0.0/12,192.168.0.0/16,.svc,.cluster.local +# The amount of time to wait for a response from the server before canceling the +# request. Used to retrieve the latest commit of configured git repositories. +gitClientTimeout: 30s + bootstrap: enabled: true # The namespace that will be autocreated and the local cluster will be registered in diff --git a/integrationtests/gitjob/controller/controller_test.go b/integrationtests/gitjob/controller/controller_test.go index c2eb6277b3..30baa11781 100644 --- a/integrationtests/gitjob/controller/controller_test.go +++ b/integrationtests/gitjob/controller/controller_test.go @@ -9,13 +9,6 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - "sigs.k8s.io/controller-runtime/pkg/client" - - "github.com/rancher/fleet/integrationtests/utils" - v1alpha1 "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1" - "github.com/rancher/wrangler/v3/pkg/genericcondition" - "github.com/rancher/wrangler/v3/pkg/name" - batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" @@ -23,6 +16,13 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/util/retry" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/rancher/wrangler/v3/pkg/genericcondition" + "github.com/rancher/wrangler/v3/pkg/name" + + "github.com/rancher/fleet/integrationtests/utils" + v1alpha1 "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1" ) const ( @@ -51,7 +51,6 @@ func checkCondition(gitrepo *v1alpha1.GitRepo, condType string, status corev1.Co } var _ = Describe("GitJob controller", func() { - When("a new GitRepo is created", func() { var ( gitRepo v1alpha1.GitRepo @@ -614,7 +613,6 @@ var _ = Describe("GitRepo", func() { }) var _ = Describe("GitRepo Status Fields", func() { - var ( gitrepo *v1alpha1.GitRepo bd *v1alpha1.BundleDeployment diff --git a/integrationtests/gitjob/controller/suite_test.go b/integrationtests/gitjob/controller/suite_test.go index 310f5e91b6..6f692eb561 100644 --- a/integrationtests/gitjob/controller/suite_test.go +++ b/integrationtests/gitjob/controller/suite_test.go @@ -10,22 +10,21 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/reugn/go-quartz/quartz" - "go.uber.org/mock/gomock" + "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/rest" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/envtest" + "sigs.k8s.io/controller-runtime/pkg/log/zap" "github.com/rancher/fleet/internal/cmd/controller/gitops/reconciler" ctrlreconciler "github.com/rancher/fleet/internal/cmd/controller/reconciler" "github.com/rancher/fleet/internal/cmd/controller/target" + "github.com/rancher/fleet/internal/config" "github.com/rancher/fleet/internal/manifest" v1alpha1 "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1" "github.com/rancher/fleet/pkg/git/mocks" - - "k8s.io/client-go/kubernetes/scheme" - "k8s.io/client-go/rest" - ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/envtest" - "sigs.k8s.io/controller-runtime/pkg/log/zap" ) const ( @@ -45,7 +44,7 @@ var ( func TestGitJobController(t *testing.T) { RegisterFailHandler(Fail) - RunSpecs(t, "Fleet CLI Cleanup Suite") + RunSpecs(t, "Fleet GitJob Controller Suite") } var _ = BeforeSuite(func() { @@ -90,6 +89,8 @@ var _ = BeforeSuite(func() { sched := quartz.NewStdScheduler() Expect(sched).ToNot(BeNil()) + config.Set(&config.Config{}) + err = (&reconciler.GitJobReconciler{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), diff --git a/integrationtests/gitjob/git/git_test.go b/integrationtests/gitjob/git/git_test.go index a633fdbf64..a7baed8823 100644 --- a/integrationtests/gitjob/git/git_test.go +++ b/integrationtests/gitjob/git/git_test.go @@ -17,11 +17,10 @@ import ( dockercontainer "github.com/docker/docker/api/types/container" dockermount "github.com/docker/docker/api/types/mount" gogit "github.com/go-git/go-git/v5" - "github.com/go-git/go-git/v5/config" + gitconfig "github.com/go-git/go-git/v5/config" "github.com/go-git/go-git/v5/plumbing/object" "github.com/go-git/go-git/v5/plumbing/transport" httpgit "github.com/go-git/go-git/v5/plumbing/transport/http" - "github.com/gogits/go-gogs-client" cp "github.com/otiai10/copy" "github.com/stretchr/testify/require" @@ -29,16 +28,16 @@ import ( "github.com/testcontainers/testcontainers-go/wait" "go.uber.org/mock/gomock" "golang.org/x/crypto/ssh" - - "github.com/rancher/fleet/internal/mocks" - v1alpha1 "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1" - "github.com/rancher/fleet/pkg/git" - v1 "k8s.io/api/core/v1" kerrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "sigs.k8s.io/controller-runtime/pkg/client/fake" + + "github.com/rancher/fleet/internal/config" + "github.com/rancher/fleet/internal/mocks" + v1alpha1 "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1" + "github.com/rancher/fleet/pkg/git" ) /* @@ -111,6 +110,7 @@ func TestLatestCommit_NoAuth(t *testing.T) { }, } + config.Set(&config.Config{}) for name, test := range tests { t.Run(name, func(t *testing.T) { f := git.Fetch{} @@ -466,7 +466,7 @@ func initRepo(url string, name string, private bool) (string, error) { if err != nil { return "", err } - cfg.Remotes["upstream"] = &config.RemoteConfig{ + cfg.Remotes["upstream"] = &gitconfig.RemoteConfig{ Name: "upstream", URLs: []string{repoURL}, } @@ -529,7 +529,7 @@ func addRepoCommitAndTag(url string, name string, tag string, tagMessage string) if err != nil { return "", err } - cfg.Remotes["upstream"] = &config.RemoteConfig{ + cfg.Remotes["upstream"] = &gitconfig.RemoteConfig{ Name: "upstream", URLs: []string{url}, } @@ -574,7 +574,7 @@ func addRepoCommitAndTag(url string, name string, tag string, tagMessage string) Username: gogsUser, Password: gogsPass, }, - RefSpecs: []config.RefSpec{config.RefSpec("refs/tags/*:refs/tags/*")}, + RefSpecs: []gitconfig.RefSpec{gitconfig.RefSpec("refs/tags/*:refs/tags/*")}, }) if err != nil { return "", nil diff --git a/internal/cmd/controller/gitops/operator.go b/internal/cmd/controller/gitops/operator.go index 1a11d9021a..85009026f8 100644 --- a/internal/cmd/controller/gitops/operator.go +++ b/internal/cmd/controller/gitops/operator.go @@ -8,16 +8,8 @@ import ( "strconv" "time" - command "github.com/rancher/fleet/internal/cmd" - "github.com/rancher/fleet/internal/cmd/controller/gitops/reconciler" - "github.com/rancher/fleet/internal/metrics" - fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1" - "github.com/rancher/fleet/pkg/git" - "github.com/rancher/fleet/pkg/version" - "github.com/rancher/fleet/pkg/webhook" "github.com/reugn/go-quartz/quartz" "github.com/spf13/cobra" - "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" @@ -27,8 +19,16 @@ import ( clog "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/log/zap" metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" - "golang.org/x/sync/errgroup" + + command "github.com/rancher/fleet/internal/cmd" + "github.com/rancher/fleet/internal/cmd/controller/gitops/reconciler" + fcreconciler "github.com/rancher/fleet/internal/cmd/controller/reconciler" + "github.com/rancher/fleet/internal/metrics" + fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1" + "github.com/rancher/fleet/pkg/git" + "github.com/rancher/fleet/pkg/version" + "github.com/rancher/fleet/pkg/webhook" ) var ( @@ -121,7 +121,7 @@ func (g *GitOperator) Run(cmd *cobra.Command, args []string) error { workers = w } - reconciler := &reconciler.GitJobReconciler{ + gitJobReconciler := &reconciler.GitJobReconciler{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), Image: g.Image, @@ -132,14 +132,30 @@ func (g *GitOperator) Run(cmd *cobra.Command, args []string) error { Clock: reconciler.RealClock{}, } + configReconciler := &fcreconciler.ConfigReconciler{ + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + SystemNamespace: namespace, + ShardID: g.ShardID, + } + + if err := fcreconciler.Load(ctx, mgr.GetAPIReader(), namespace); err != nil { + setupLog.Error(err, "failed to load config") + return err + } + group, ctx := errgroup.WithContext(ctx) group.Go(func() error { return startWebhook(ctx, namespace, g.Listen, mgr.GetClient(), mgr.GetCache()) }) group.Go(func() error { - setupLog.Info("starting manager") + setupLog.Info("starting config manager") + if err = configReconciler.SetupWithManager(mgr); err != nil { + return err + } - if err = reconciler.SetupWithManager(mgr); err != nil { + setupLog.Info("starting gitops manager") + if err = gitJobReconciler.SetupWithManager(mgr); err != nil { return err } diff --git a/internal/cmd/controller/reconciler/config_controller.go b/internal/cmd/controller/reconciler/config_controller.go index 84bb779adc..e1efa47e2e 100644 --- a/internal/cmd/controller/reconciler/config_controller.go +++ b/internal/cmd/controller/reconciler/config_controller.go @@ -58,6 +58,7 @@ func (r *ConfigReconciler) SetupWithManager(mgr ctrl.Manager) error { object.GetName() == config.ManagerConfigName }), predicate.Or( + predicate.ResourceVersionChangedPredicate{}, predicate.GenerationChangedPredicate{}, predicate.AnnotationChangedPredicate{}, predicate.LabelChangedPredicate{}, diff --git a/internal/config/config.go b/internal/config/config.go index 8629327759..d7f92fe9a0 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -108,6 +108,11 @@ type Config struct { // to trust CA bundles from the operating system's store. If set to `strict`, then the agent shall only connect // to a server which uses the exact CA configured when creating/updating the agent. AgentTLSMode string `json:"agentTLSMode,omitempty"` + + // The amount of time to wait for a response from the server before + // canceling the request. Used to retrieve the latest commit of configured + // git repositories. + GitClientTimeout metav1.Duration `json:"gitClientTimeout,omitempty"` } type Bootstrap struct { diff --git a/pkg/git/fetch.go b/pkg/git/fetch.go index 708cf73d9d..b141973508 100644 --- a/pkg/git/fetch.go +++ b/pkg/git/fetch.go @@ -3,6 +3,7 @@ package git import ( "context" + "github.com/rancher/fleet/internal/config" v1alpha1 "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1" corev1 "k8s.io/api/core/v1" @@ -46,6 +47,7 @@ func (f *Fetch) LatestCommit(ctx context.Context, gitrepo *v1alpha1.GitRepo, cli CABundle: gitrepo.Spec.CABundle, Credential: &secret, InsecureTLSVerify: gitrepo.Spec.InsecureSkipTLSverify, + Timeout: config.Get().GitClientTimeout.Duration, log: log.FromContext(ctx), }) if err != nil { diff --git a/pkg/git/fetch_test.go b/pkg/git/fetch_test.go index 8aa4f437cf..f9341529ea 100644 --- a/pkg/git/fetch_test.go +++ b/pkg/git/fetch_test.go @@ -8,6 +8,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/rancher/fleet/internal/config" fleetv1 "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1" "github.com/rancher/fleet/pkg/git" corev1 "k8s.io/api/core/v1" @@ -76,6 +77,9 @@ var _ = Describe("git fetch's LatestCommit tests", func() { }) It("returns the commit for the expected revision", func() { + config.Set(&config.Config{ + GitClientTimeout: metav1.Duration{Duration: 0}, + }) secret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "test-secret", diff --git a/pkg/git/netutils.go b/pkg/git/netutils.go index 2b399dac89..be7c0b5e6e 100644 --- a/pkg/git/netutils.go +++ b/pkg/git/netutils.go @@ -16,8 +16,6 @@ import ( corev1 "k8s.io/api/core/v1" ) -var clientTimeout = time.Duration(30) - // GetAuthFromSecret returns the AuthMethod calculated from the given secret // The credentials secret is expected to be either basic-auth or ssh-auth (with extra known_hosts data option) func GetAuthFromSecret(url string, creds *corev1.Secret) (transport.AuthMethod, error) { @@ -59,7 +57,7 @@ func GetAuthFromSecret(url string, creds *corev1.Secret) (transport.AuthMethod, // GetHTTPClientFromSecret returns a HTTP client filled from the information in the given secret // and optional CABundle and insecureTLSVerify -func GetHTTPClientFromSecret(creds *corev1.Secret, CABundle []byte, insecureTLSVerify bool) (*http.Client, error) { +func GetHTTPClientFromSecret(creds *corev1.Secret, CABundle []byte, insecureTLSVerify bool, timeout time.Duration) (*http.Client, error) { var ( username string password string @@ -99,7 +97,7 @@ func GetHTTPClientFromSecret(creds *corev1.Secret, CABundle []byte, insecureTLSV client := &http.Client{ Transport: transport, - Timeout: clientTimeout * time.Second, + Timeout: timeout, } if username != "" || password != "" { client.Transport = &basicRoundTripper{ diff --git a/pkg/git/netutils_test.go b/pkg/git/netutils_test.go index 65064bf6a0..331f782a92 100644 --- a/pkg/git/netutils_test.go +++ b/pkg/git/netutils_test.go @@ -3,6 +3,7 @@ package git_test import ( "encoding/pem" "net/http" + "time" httpgit "github.com/go-git/go-git/v5/plumbing/transport/http" gossh "github.com/go-git/go-git/v5/plumbing/transport/ssh" @@ -13,6 +14,8 @@ import ( corev1 "k8s.io/api/core/v1" ) +const gitClientTimeout = time.Second * 30 + var _ = Describe("git's GetAuthFromSecret tests", func() { var ( secret *corev1.Secret @@ -252,7 +255,7 @@ YcwLYudAztZeA/A4aM5Y0MA6PlNIeoHohuMkSZNOBcvkNEWdzGBpKb34yLfMarNm var _ = Describe("git's GetHTTPClientFromSecret tests", func() { When("using a nil secret, no caBudle and InsecureSkipVerify = false", func() { var caBundle []byte - client, err := git.GetHTTPClientFromSecret(nil, caBundle, false) + client, err := git.GetHTTPClientFromSecret(nil, caBundle, false, gitClientTimeout) Expect(err).ToNot(HaveOccurred()) Expect(client).ToNot(BeNil()) expectedTransport, ok := client.Transport.(*http.Transport) @@ -278,7 +281,7 @@ var _ = Describe("git's GetHTTPClientFromSecret tests", func() { When("using a nil secret, no caBudle and InsecureSkipVerify = true", func() { var caBundle []byte - client, err := git.GetHTTPClientFromSecret(nil, caBundle, true) + client, err := git.GetHTTPClientFromSecret(nil, caBundle, true, gitClientTimeout) Expect(err).ToNot(HaveOccurred()) Expect(client).ToNot(BeNil()) expectedTransport, ok := client.Transport.(*http.Transport) @@ -320,7 +323,7 @@ DXZDjC5Ty3zfDBeWUA== block, _ := pem.Decode([]byte(caBundlePEM)) Expect(block).ToNot(BeNil()) - client, err := git.GetHTTPClientFromSecret(nil, block.Bytes, true) + client, err := git.GetHTTPClientFromSecret(nil, block.Bytes, true, gitClientTimeout) Expect(err).ToNot(HaveOccurred()) Expect(client).ToNot(BeNil()) expectedTransport, ok := client.Transport.(*http.Transport) @@ -348,7 +351,7 @@ DXZDjC5Ty3zfDBeWUA== caBundle := []byte(`-----BEGIN CERTIFICATE----- SUPER FAKE CERT -----END CERTIFICATE-----`) - client, err := git.GetHTTPClientFromSecret(nil, caBundle, true) + client, err := git.GetHTTPClientFromSecret(nil, caBundle, true, gitClientTimeout) It("returns an error", func() { Expect(err).To(HaveOccurred()) Expect(client).To(BeNil()) @@ -366,7 +369,7 @@ SUPER FAKE CERT corev1.BasicAuthPasswordKey: password, }, } - client, err := git.GetHTTPClientFromSecret(secret, nil, false) + client, err := git.GetHTTPClientFromSecret(secret, nil, false, gitClientTimeout) Expect(err).ToNot(HaveOccurred()) Expect(client).ToNot(BeNil()) @@ -403,7 +406,7 @@ MC4CAQAwBQYDK2VwBCIEINifzf07d9qx3d44e0FSbV4mC/xQxT644RRbpgNpin7I corev1.TLSPrivateKeyKey: []byte(keyPEM), }, } - client, err := git.GetHTTPClientFromSecret(secret, nil, false) + client, err := git.GetHTTPClientFromSecret(secret, nil, false, gitClientTimeout) Expect(err).ToNot(HaveOccurred()) Expect(client).ToNot(BeNil()) @@ -431,7 +434,7 @@ THIS IS NOT A VALID KEY corev1.TLSPrivateKeyKey: []byte(keyPEM), }, } - _, err := git.GetHTTPClientFromSecret(secret, nil, false) + _, err := git.GetHTTPClientFromSecret(secret, nil, false, gitClientTimeout) It("returns an error", func() { Expect(err).To(HaveOccurred()) diff --git a/pkg/git/remote.go b/pkg/git/remote.go index 11ddefe4c0..3f00c97f16 100644 --- a/pkg/git/remote.go +++ b/pkg/git/remote.go @@ -2,6 +2,7 @@ package git import ( "fmt" + "time" gogit "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/config" @@ -16,6 +17,7 @@ type options struct { CABundle []byte InsecureTLSVerify bool Headers map[string]string + Timeout time.Duration log logr.Logger } diff --git a/pkg/git/vendor.go b/pkg/git/vendor.go index cee68a1fd1..e9c6c1cdf5 100644 --- a/pkg/git/vendor.go +++ b/pkg/git/vendor.go @@ -56,7 +56,12 @@ func getRancherCommitsURL(url *neturl.URL, branch string) string { // latestCommitFromCommitsURL returns the latest commit using the given commits url func latestCommitFromCommitsURL(commitsUrl string, opts *options) (string, error) { - client, err := GetHTTPClientFromSecret(opts.Credential, opts.CABundle, opts.InsecureTLSVerify) + client, err := GetHTTPClientFromSecret( + opts.Credential, + opts.CABundle, + opts.InsecureTLSVerify, + opts.Timeout, + ) if err != nil { return "", err } diff --git a/pkg/git/vendor_test.go b/pkg/git/vendor_test.go index 5c247d94fc..0daa5108c1 100644 --- a/pkg/git/vendor_test.go +++ b/pkg/git/vendor_test.go @@ -77,25 +77,25 @@ var _ = Describe("git's vendor specific functions tests", func() { }) It("returns an error when the server timeouts", func() { - clientTimeout = 1 + clientTimeout := time.Second svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - time.Sleep(time.Second*clientTimeout + 1) + time.Sleep(clientTimeout + 1) w.WriteHeader(http.StatusGatewayTimeout) })) - commit, err := latestCommitFromCommitsURL(svr.URL, &options{}) + commit, err := latestCommitFromCommitsURL(svr.URL, &options{Timeout: clientTimeout}) Expect(err).To(HaveOccurred()) Expect(commit).To(BeEmpty()) }) It("returns no error when cannot get a valid client, and changed returned is true", func() { - clientTimeout = 1 + clientTimeout := time.Second svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusGatewayTimeout) })) caBundle := []byte(`-----BEGIN CERTIFICATE----- SUPER FAKE CERT -----END CERTIFICATE-----`) - commit, err := latestCommitFromCommitsURL(svr.URL, &options{CABundle: caBundle}) + commit, err := latestCommitFromCommitsURL(svr.URL, &options{CABundle: caBundle, Timeout: clientTimeout}) // no error and returns true, so the client is forced to run the List to get results Expect(err).To(HaveOccurred()) Expect(err.Error()).To(Equal("x509: malformed certificate"))