Skip to content

Commit

Permalink
Prevent 0 as value for gitClientTimeout
Browse files Browse the repository at this point in the history
A zero value for the gitClientTimeout setting would mean that the
request would not have a client side timeout set, potentially resulting
in an endless waiting state. Although it should not happen that this
value is missing, which would lead to it being zero, it should be safer
to fall back to 30s as in previous versions of Fleet, thereby preventing
the user from disabling the git client timeout completely.

Follow-up for #2188
  • Loading branch information
p-se committed Sep 3, 2024
1 parent 056b443 commit e86a273
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 7 deletions.
1 change: 1 addition & 0 deletions charts/fleet/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ 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.
# A non-existent value or 0 will result in a timeout of 30 seconds.
gitClientTimeout: 30s

bootstrap:
Expand Down
17 changes: 10 additions & 7 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@ import (
"context"
"encoding/json"
"sync"

"github.com/rancher/fleet/pkg/version"
"time"

corev1 "github.com/rancher/wrangler/v3/pkg/generated/controllers/core/v1"

v1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/yaml"

"github.com/rancher/fleet/pkg/version"
)

const (
Expand Down Expand Up @@ -44,8 +44,9 @@ const (
)

var (
DefaultManagerImage = "rancher/fleet" + ":" + version.Version
DefaultAgentImage = "rancher/fleet-agent" + ":" + version.Version
DefaultManagerImage = "rancher/fleet" + ":" + version.Version
DefaultAgentImage = "rancher/fleet-agent" + ":" + version.Version
DefaultGitClientTimeout = metav1.Duration{Duration: 30 * time.Second}

config *Config
callbacks = map[int]func(*Config) error{}
Expand Down Expand Up @@ -111,7 +112,8 @@ type Config struct {

// 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.
// git repositories. A non-existent value or 0 will result in a timeout of
// 30 seconds.
GitClientTimeout metav1.Duration `json:"gitClientTimeout,omitempty"`

// GarbageCollectionInterval determines how often agents clean up obsolete Helm releases.
Expand Down Expand Up @@ -199,7 +201,8 @@ func Lookup(_ context.Context, namespace, name string, configMaps corev1.ConfigM

func DefaultConfig() *Config {
return &Config{
AgentImage: DefaultAgentImage,
AgentImage: DefaultAgentImage,
GitClientTimeout: DefaultGitClientTimeout,
}
}

Expand Down
34 changes: 34 additions & 0 deletions internal/config/config_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package config_test

import (
"time"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

v1 "k8s.io/api/core/v1"

"github.com/rancher/fleet/internal/config"
)

var _ = Describe("Config", func() {
When("not having set a value for gitClientTimeout", func() {
It("should return the default value", func() {
cfg, err := config.ReadConfig(&v1.ConfigMap{Data: map[string]string{}})
Expect(err).ToNot(HaveOccurred())
Expect(cfg.GitClientTimeout.Duration).To(Equal(30 * time.Second))
})
})
When("having set a value for gitClientTimeout", func() {
It("should return the set value", func() {
jsonConfig := `{"gitClientTimeout": "20s"}`
cfg, err := config.ReadConfig(&v1.ConfigMap{
Data: map[string]string{
"config": jsonConfig,
},
})
Expect(err).ToNot(HaveOccurred())
Expect(cfg.GitClientTimeout.Duration).To(Equal(20 * time.Second))
})
})
})
22 changes: 22 additions & 0 deletions internal/config/suite_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package config_test

import (
"testing"
"time"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)

const (
timeout = 30 * time.Second
)

func TestFleet(t *testing.T) {
RegisterFailHandler(Fail)
RunSpecs(t, "Config Suite")
}

var _ = BeforeSuite(func() {
SetDefaultEventuallyTimeout(timeout)
})

0 comments on commit e86a273

Please sign in to comment.