feat(warehouse): allow discovery interval and limit configurations (#…

…2038)

Signed-off-by: Hidde Beydals <hidde@hhh.computer>

api/v1alpha1/warehouse_types.go

@@ -55,6 +55,15 @@ type WarehouseSpec struct {
 	// empty, the defaulting webhook will set the value of this field to the value
 	// of the shard label.
 	Shard string `json:"shard,omitempty" protobuf:"bytes,2,opt,name=shard"`
+	// Interval is the reconciliation interval for this Warehouse. On each
+	// reconciliation, the Warehouse will discover new artifacts and optionally
+	// produce new Freight. This field is optional. When left unspecified, the
+	// field is implicitly treated as if its value were "5m0s".
+	//
+	// +kubebuilder:validation:Type=string
+	// +kubebuilder:validation:Pattern="^([0-9]+(\\.[0-9]+)?(s|m|h))+$"
+	// +kubebuilder:default="5m0s"
+	Interval metav1.Duration `json:"interval" protobuf:"bytes,4,opt,name=interval"`
 	// FreightCreationPolicy describes how Freight is created by this Warehouse.
 	// This field is optional. When left unspecified, the field is implicitly
 	// treated as if its value were "Automatic".
@@ -172,6 +181,16 @@ type GitSubscription struct {
 	// subset of them.
 	// +kubebuilder:validation:Optional
 	ExcludePaths []string `json:"excludePaths,omitempty" protobuf:"bytes,9,rep,name=excludePaths"`
+	// DiscoveryLimit is an optional limit on the number of commits that can be
+	// discovered for this subscription. The limit is applied after filtering
+	// commits based on the AllowTags and IgnoreTags fields.
+	// When left unspecified, the field is implicitly treated as if its value
+	// were "20". The upper limit for this field is 100.
+	//
+	// +kubebuilder:validation:Minimum=1
+	// +kubebuilder:validation:Maximum=100
+	// +kubebuilder:default=20
+	DiscoveryLimit int32 `json:"discoveryLimit,omitempty" protobuf:"varint,10,opt,name=discoveryLimit"`
 }
 
 // ImageSubscription defines a subscription to an image repository.
@@ -237,6 +256,16 @@ type ImageSubscription struct {
 	// should be ignored when connecting to the repository. This should be enabled
 	// only with great caution.
 	InsecureSkipTLSVerify bool `json:"insecureSkipTLSVerify,omitempty" protobuf:"varint,8,opt,name=insecureSkipTLSVerify"`
+	// DiscoveryLimit is an optional limit on the number of image references
+	// that can be discovered for this subscription. The limit is applied after
+	// filtering images based on the AllowTags and IgnoreTags fields.
+	// When left unspecified, the field is implicitly treated as if its value
+	// were "20". The upper limit for this field is 100.
+	//
+	// +kubebuilder:validation:Minimum=1
+	// +kubebuilder:validation:Maximum=100
+	// +kubebuilder:default=20
+	DiscoveryLimit int32 `json:"discoveryLimit,omitempty" protobuf:"varint,9,opt,name=discoveryLimit"`
 }
 
 // ChartSubscription defines a subscription to a Helm chart repository.
@@ -267,6 +296,16 @@ type ChartSubscription struct {
 	//
 	// +kubebuilder:validation:Optional
 	SemverConstraint string `json:"semverConstraint,omitempty" protobuf:"bytes,3,opt,name=semverConstraint"`
+	// DiscoveryLimit is an optional limit on the number of chart versions that
+	// can be discovered for this subscription. The limit is applied after
+	// filtering charts based on the SemverConstraint field.
+	// When left unspecified, the field is implicitly treated as if its value
+	// were "20". The upper limit for this field is 100.
+	//
+	// +kubebuilder:validation:Minimum=1
+	// +kubebuilder:validation:Maximum=100
+	// +kubebuilder:default=20
+	DiscoveryLimit int32 `json:"discoveryLimit,omitempty" protobuf:"varint,4,opt,name=discoveryLimit"`
 }
 
 // WarehouseStatus describes a Warehouse's most recently observed state.

charts/kargo/resources/crds/kargo.akuity.io_warehouses.yaml

@@ -56,6 +56,15 @@ spec:
                 - Automatic
                 - Manual
                 type: string
+              interval:
+                default: 5m0s
+                description: |-
+                  Interval is the reconciliation interval for this Warehouse. On each
+                  reconciliation, the Warehouse will discover new artifacts and optionally
+                  produce new Freight. This field is optional. When left unspecified, the
+                  field is implicitly treated as if its value were "5m0s".
+                pattern: ^([0-9]+(\.[0-9]+)?(s|m|h))+$
+                type: string
               shard:
                 description: |-
                   Shard is the name of the shard that this Warehouse belongs to. This is an
@@ -80,6 +89,18 @@ spec:
                       description: Chart describes a subscription to a Helm chart
                         repository.
                       properties:
+                        discoveryLimit:
+                          default: 20
+                          description: |-
+                            DiscoveryLimit is an optional limit on the number of chart versions that
+                            can be discovered for this subscription. The limit is applied after
+                            filtering charts based on the SemverConstraint field.
+                            When left unspecified, the field is implicitly treated as if its value
+                            were "20". The upper limit for this field is 100.
+                          format: int32
+                          maximum: 100
+                          minimum: 1
+                          type: integer
                         name:
                           description: |-
                             Name specifies the name of a Helm chart to subscribe to within a classic
@@ -146,6 +167,18 @@ spec:
                           - NewestTag
                           - SemVer
                           type: string
+                        discoveryLimit:
+                          default: 20
+                          description: |-
+                            DiscoveryLimit is an optional limit on the number of commits that can be
+                            discovered for this subscription. The limit is applied after filtering
+                            commits based on the AllowTags and IgnoreTags fields.
+                            When left unspecified, the field is implicitly treated as if its value
+                            were "20". The upper limit for this field is 100.
+                          format: int32
+                          maximum: 100
+                          minimum: 1
+                          type: integer
                         excludePaths:
                           description: |-
                             ExcludePaths is a list of selectors that designate paths in the repository
@@ -226,6 +259,18 @@ spec:
                             image tags that are considered in determining the newest version of an
                             image. This field is optional.
                           type: string
+                        discoveryLimit:
+                          default: 20
+                          description: |-
+                            DiscoveryLimit is an optional limit on the number of image references
+                            that can be discovered for this subscription. The limit is applied after
+                            filtering images based on the AllowTags and IgnoreTags fields.
+                            When left unspecified, the field is implicitly treated as if its value
+                            were "20". The upper limit for this field is 100.
+                          format: int32
+                          maximum: 100
+                          minimum: 1
+                          type: integer
                         gitRepoURL:
                           description: |-
                             GitRepoURL optionally specifies the URL of a Git repository that contains
@@ -300,6 +345,7 @@ spec:
                 type: array
             required:
             - freightCreationPolicy
+            - interval
             - subscriptions
             type: object
           status:

internal/controller/warehouses/git.go

@@ -90,9 +90,13 @@ func (r *reconciler) discoverCommits(
 
 			for _, meta := range tags {
 				discovered = append(discovered, kargoapi.DiscoveredCommit{
-					ID:          meta.CommitID,
-					Tag:         meta.Tag,
-					Subject:     meta.Subject,
+					ID:  meta.CommitID,
+					Tag: meta.Tag,
+					// A decent subject length for a commit message is 50 characters
+					// (based on the 50/72 rule). We are nice people, and allow a
+					// bit more. But not an excessive amount, to minimize the risk of
+					// exceeding the maximum size of the object in the API server.
+					Subject:     shortenString(meta.Subject, 80),
 					Author:      meta.Author,
 					Committer:   meta.Committer,
 					CreatorDate: &metav1.Time{Time: meta.CreatorDate},
@@ -106,9 +110,13 @@ func (r *reconciler) discoverCommits(
 
 			for _, meta := range commits {
 				discovered = append(discovered, kargoapi.DiscoveredCommit{
-					ID:          meta.ID,
-					Branch:      sub.Branch,
-					Subject:     meta.Subject,
+					ID:     meta.ID,
+					Branch: sub.Branch,
+					// A decent subject length for a commit message is 50 characters
+					// (based on the 50/72 rule). We are nice people, and allow a
+					// bit more. But not an excessive amount, to minimize the risk of
+					// exceeding the maximum size of the object in the API server.
+					Subject:     shortenString(meta.Subject, 80),
 					Author:      meta.Author,
 					Committer:   meta.Committer,
 					CreatorDate: &metav1.Time{Time: meta.CommitDate},
@@ -126,10 +134,10 @@ func (r *reconciler) discoverCommits(
 }
 
 func (r *reconciler) discoverBranchHistory(repo git.Repo, sub kargoapi.GitSubscription) ([]git.CommitMetadata, error) {
-	const limit = 20
+	limit := int(sub.DiscoveryLimit)
 	var filteredCommits = make([]git.CommitMetadata, 0, limit)
-	for skip := uint(0); ; skip += limit {
-		commits, err := r.listCommitsFn(repo, limit, skip)
+	for skip := uint(0); ; skip += uint(limit) {
+		commits, err := r.listCommitsFn(repo, uint(limit), skip)
 		if err != nil {
 			return nil, fmt.Errorf("error listing commits from git repo %q: %w", sub.RepoURL, err)
 		}
@@ -223,7 +231,7 @@ func (r *reconciler) discoverTags(repo git.Repo, sub kargoapi.GitSubscription) (
 
 	// If no include or exclude paths are specified, return the first tags up to
 	// the limit.
-	const limit = 20
+	limit := int(sub.DiscoveryLimit)
 	if len(tags) == 0 || (sub.IncludePaths == nil && sub.ExcludePaths == nil) {
 		return trimSlice(tags, limit), nil
 	}
@@ -441,3 +449,12 @@ func (r *reconciler) listTags(repo git.Repo) ([]git.TagMetadata, error) {
 func (r *reconciler) getDiffPathsForCommitID(repo git.Repo, commitID string) ([]string, error) {
 	return repo.GetDiffPathsForCommitID(commitID)
 }
+
+// shortenString truncates the given string to the given length, appending an
+// ellipsis if the string is longer than the length.
+func shortenString(str string, length int) string {
+	if length >= 0 && len(str) > length {
+		return str[:length] + "..."
+	}
+	return str
+}

internal/controller/warehouses/git_test.go

@@ -468,28 +468,25 @@ func TestDiscoverTags(t *testing.T) {
 			name: "more tags than limit",
 			sub: kargoapi.GitSubscription{
 				CommitSelectionStrategy: kargoapi.CommitSelectionStrategyNewestTag,
+				DiscoveryLimit:          3,
 			},
 			reconciler: &reconciler{
 				listTagsFn: func(git.Repo) ([]git.TagMetadata, error) {
 					return []git.TagMetadata{
 						{Tag: "a"}, {Tag: "b"}, {Tag: "c"}, {Tag: "d"}, {Tag: "e"},
-						{Tag: "f"}, {Tag: "g"}, {Tag: "h"}, {Tag: "i"}, {Tag: "j"},
-						{Tag: "k"}, {Tag: "l"}, {Tag: "m"}, {Tag: "n"}, {Tag: "o"},
-						{Tag: "p"}, {Tag: "q"}, {Tag: "r"}, {Tag: "s"}, {Tag: "t"},
-						{Tag: "u"}, {Tag: "v"}, {Tag: "w"}, {Tag: "x"}, {Tag: "y"},
-						{Tag: "z"},
 					}, nil
 				},
 			},
 			assertions: func(t *testing.T, tags []git.TagMetadata, err error) {
 				require.NoError(t, err)
-				require.Len(t, tags, 20)
+				require.Len(t, tags, 3)
 			},
 		},
 		{
 			name: "with path filters",
 			sub: kargoapi.GitSubscription{
-				IncludePaths: []string{regexpPrefix + "^.*third_path_to_a/file$"},
+				IncludePaths:   []string{regexpPrefix + "^.*third_path_to_a/file$"},
+				DiscoveryLimit: 20,
 			},
 			reconciler: &reconciler{
 				listTagsFn: func(git.Repo) ([]git.TagMetadata, error) {
@@ -1012,3 +1009,42 @@ func TestMatchesPathsFilters(t *testing.T) {
 		})
 	}
 }
+
+func TestShortenString(t *testing.T) {
+	testCases := []struct {
+		name   string
+		str    string
+		length int
+		want   string
+	}{
+		{
+			name:   "exceeds length",
+			str:    "this is a long string",
+			length: 10,
+			want:   "this is a ...",
+		},
+		{
+			name:   "equal length",
+			str:    "this is a long string",
+			length: 21,
+			want:   "this is a long string",
+		},
+		{
+			name:   "shorter length",
+			str:    "this is a long string",
+			length: 30,
+			want:   "this is a long string",
+		},
+		{
+			name:   "empty string",
+			str:    "",
+			length: 10,
+			want:   "",
+		},
+	}
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			require.Equal(t, testCase.want, shortenString(testCase.str, testCase.length))
+		})
+	}
+}

internal/controller/warehouses/helm.go

@@ -80,7 +80,7 @@ func (r *reconciler) discoverCharts(
 			RepoURL:          sub.RepoURL,
 			Name:             sub.Name,
 			SemverConstraint: sub.SemverConstraint,
-			Versions:         trimSlice(versions, 20),
+			Versions:         trimSlice(versions, int(sub.DiscoveryLimit)),
 		})
 	}
 

internal/controller/warehouses/images.go

@@ -140,7 +140,7 @@ func imageSelectorForSubscription(
 			Platform:              sub.Platform,
 			Creds:                 creds,
 			InsecureSkipTLSVerify: sub.InsecureSkipTLSVerify,
-			DiscoveryLimit:        20,
+			DiscoveryLimit:        int(sub.DiscoveryLimit),
 		},
 	)
 }