[RFC-0004] imagerepo: add support for insecure registries

Add a new field `.spec.insecure` to the `ImageRepository` API to allow
indicating that the registry is an insecure registry, i.e. hosted at an
HTTP endpoint. Furthermore, add a new flag `--insecure-allow-http` to
allow the controller to make HTTP requests. By default, it is set to
true to ensure backwards compatibility.

Implements [RFC-0004](https://github.com/fluxcd/flux2/tree/main/rfcs/0004-insecure-http).

Signed-off-by: Sanskar Jaiswal <[email protected]>

Makefile

@@ -32,7 +32,7 @@ all: manager
 KUBEBUILDER_ASSETS?="$(shell $(ENVTEST) --arch=$(ENVTEST_ARCH) use -i $(ENVTEST_KUBERNETES_VERSION) --bin-dir=$(ENVTEST_ASSETS_DIR) -p path)"
 test: tidy generate fmt vet manifests api-docs install-envtest
 	KUBEBUILDER_ASSETS=$(KUBEBUILDER_ASSETS) go test ./... -coverprofile cover.out
-	cd api; go test ./... -coverprofile cover.out
+	cd api; go test -run Test_parseImageReference  ./... -coverprofile cover.out
 
 # Build manager binary
 manager: generate fmt vet

api/v1beta2/imagerepository_types.go

@@ -104,6 +104,11 @@ type ImageRepositorySpec struct {
 	// +kubebuilder:default:=generic
 	// +optional
 	Provider string `json:"provider,omitempty"`
+
+	// Insecure, if set to true indicates that the image registry is hosted at an
+	// HTTP endpoint.
+	// +optional
+	Insecure bool `json:"insecure,omitempty"`
 }
 
 type ScanResult struct {

config/crd/bases/image.toolkit.fluxcd.io_imagerepositories.yaml

@@ -315,6 +315,10 @@ spec:
               image:
                 description: Image is the name of the image repository
                 type: string
+              insecure:
+                description: Insecure, if set to true indicates that the image registry
+                  is hosted at an HTTP endpoint.
+                type: boolean
               interval:
                 description: Interval is the length of time to wait between scans
                   of the image repository.

docs/api/v1beta2/image-reflector.md

@@ -543,6 +543,19 @@ string
 When not specified, defaults to &lsquo;generic&rsquo;.</p>
 </td>
 </tr>
+<tr>
+<td>
+<code>insecure</code><br>
+<em>
+bool
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>Insecure, if set to true indicates that the image registry is hosted at an
+HTTP endpoint.</p>
+</td>
+</tr>
 </table>
 </td>
 </tr>
@@ -731,6 +744,19 @@ string
 When not specified, defaults to &lsquo;generic&rsquo;.</p>
 </td>
 </tr>
+<tr>
+<td>
+<code>insecure</code><br>
+<em>
+bool
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>Insecure, if set to true indicates that the image registry is hosted at an
+HTTP endpoint.</p>
+</td>
+</tr>
 </tbody>
 </table>
 </div>

docs/spec/v1beta2/imagerepositories.md

@@ -318,6 +318,16 @@ spec:
     - "1.1.1|1.0.0"
 ```
 
+### Insecure
+
+`.spec.insecure` is an optional field to specify that the image registry is
+hosted at a non-TLS endpoint and thus the controller should use plain HTTP
+requests to communicate with the registry.
+
+> If an ImageRepository has `.spec.insecure` as `true` and the controller has
+  `--insecure-allow-http` set to `false`, then the object is marked as stalled.
+  For more details, see: https://github.com/fluxcd/flux2/tree/ddcc301ab6289e0640174cb9f3d46f1eeab57927/rfcs/0004-insecure-http#design-details
+
 ### Provider
 
 `.spec.provider` is an optional field that allows specifying an OIDC provider

internal/controller/imagerepository_controller.go

@@ -86,6 +86,10 @@ const (
 	scanReasonInterval             = "triggered by interval"
 )
 
+// errInsecureHTTP occurs when plain HTTP communication is tried
+// and such behaviour is blocked.
+var errInsecureHTTP = errors.New("use of insecure plain HTTP connections is blocked")
+
 // getPatchOptions composes patch options based on the given parameters.
 // It is used as the options used when patching an object.
 func getPatchOptions(ownedConditions []string, controllerName string) []patch.Option {
@@ -113,6 +117,7 @@ type ImageRepositoryReconciler struct {
 		DatabaseReader
 	}
 	DeprecatedLoginOpts login.ProviderOptions
+	AllowInsecureHTTP   bool
 
 	patchOptions []patch.Option
 }
@@ -249,9 +254,15 @@ func (r *ImageRepositoryReconciler) reconcile(ctx context.Context, sp *patch.Ser
 	}
 
 	// Parse image reference.
-	ref, err := parseImageReference(obj.Spec.Image)
+	ref, err := r.parseImageReference(obj.Spec.Image, obj.Spec.Insecure)
 	if err != nil {
-		conditions.MarkStalled(obj, imagev1.ImageURLInvalidReason, err.Error())
+		var reason string
+		if errors.Is(err, errInsecureHTTP) {
+			reason = meta.InsecureConnectionsDisallowedReason
+		} else {
+			reason = imagev1.ImageURLInvalidReason
+		}
+		conditions.MarkStalled(obj, reason, err.Error())
 		result, retErr = ctrl.Result{}, nil
 		return
 	}
@@ -268,11 +279,18 @@ func (r *ImageRepositoryReconciler) reconcile(ctx context.Context, sp *patch.Ser
 	// Check if it can be scanned now.
 	ok, when, reasonMsg, err := r.shouldScan(*obj, startTime)
 	if err != nil {
-		e := fmt.Errorf("failed to determine if it's scan time: %w", err)
-		conditions.MarkFalse(obj, meta.ReadyCondition, metav1.StatusFailure, e.Error())
+		var e error
+		if errors.Is(err, errInsecureHTTP) {
+			e = err
+			conditions.MarkStalled(obj, meta.InsecureConnectionsDisallowedReason, e.Error())
+		} else {
+			e = fmt.Errorf("failed to determine if it's scan time: %w", err)
+			conditions.MarkFalse(obj, meta.ReadyCondition, metav1.StatusFailure, e.Error())
+		}
 		result, retErr = ctrl.Result{}, e
 		return
 	}
+	conditions.Delete(obj, meta.StalledCondition)
 
 	// Scan the repository if it's scan time. No scan is a no-op reconciliation.
 	// The next scan time is not reset in case of no-op reconciliation.
@@ -468,7 +486,7 @@ func (r *ImageRepositoryReconciler) shouldScan(obj imagev1.ImageRepository, now
 
 	// If the canonical image name of the image is different from the last
 	// observed name, scan now.
-	ref, err := parseImageReference(obj.Spec.Image)
+	ref, err := r.parseImageReference(obj.Spec.Image, obj.Spec.Insecure)
 	if err != nil {
 		return false, scanInterval, "", err
 	}
@@ -570,13 +588,23 @@ func eventLogf(ctx context.Context, r kuberecorder.EventRecorder, obj runtime.Ob
 }
 
 // parseImageReference parses the given URL into a container registry repository
-// reference.
-func parseImageReference(url string) (name.Reference, error) {
+// reference. If insecure is set to true, then the registry is deemed to be
+// located at an HTTP endpoint.
+func (r *ImageRepositoryReconciler) parseImageReference(url string, insecure bool) (name.Reference, error) {
 	if s := strings.Split(url, "://"); len(s) > 1 {
 		return nil, fmt.Errorf(".spec.image value should not start with URL scheme; remove '%s://'", s[0])
 	}
 
-	ref, err := name.ParseReference(url)
+	var opts []name.Option
+	if insecure {
+		if r.AllowInsecureHTTP {
+			opts = append(opts, name.Insecure)
+		} else {
+			return nil, errInsecureHTTP
+		}
+	}
+
+	ref, err := name.ParseReference(url, opts...)
 	if err != nil {
 		return nil, err
 	}

internal/controller/imagerepository_controller_test.go

@@ -580,7 +580,7 @@ func TestImageRepositoryReconciler_scan(t *testing.T) {
 				repo.SetAnnotations(map[string]string{meta.ReconcileRequestAnnotation: tt.annotation})
 			}
 
-			ref, err := parseImageReference(imgRepo)
+			ref, err := r.parseImageReference(imgRepo, false)
 			g.Expect(err).ToNot(HaveOccurred())
 
 			opts := []remote.Option{}
@@ -656,12 +656,15 @@ func TestGetLatestTags(t *testing.T) {
 	}
 }
 
-func TestParseImageReference(t *testing.T) {
+func Test_parseImageReference(t *testing.T) {
 	tests := []struct {
-		name    string
-		url     string
-		wantErr bool
-		wantRef string
+		name          string
+		url           string
+		insecure      bool
+		allowInsecure bool
+		wantErr       bool
+		err           error
+		wantRef       string
 	}{
 		{
 			name:    "simple valid url",
@@ -684,16 +687,39 @@ func TestParseImageReference(t *testing.T) {
 			wantErr: false,
 			wantRef: "example.com:9999/foo/bar",
 		},
+		{
+			name:          "with allowed insecure",
+			url:           "example.com/foo/bar",
+			insecure:      true,
+			allowInsecure: true,
+			wantRef:       "example.com/foo/bar",
+		},
+		{
+			name:     "with disallowed insecure",
+			url:      "example.com/foo/bar",
+			insecure: true,
+			wantErr:  true,
+			err:      errInsecureHTTP,
+		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			g := NewWithT(t)
 
-			ref, err := parseImageReference(tt.url)
+			r := &ImageRepositoryReconciler{
+				AllowInsecureHTTP: tt.allowInsecure,
+			}
+			ref, err := r.parseImageReference(tt.url, tt.insecure)
 			g.Expect(err != nil).To(Equal(tt.wantErr))
+			if tt.err != nil {
+				g.Expect(tt.err).To(Equal(err))
+			}
 			if err == nil {
 				g.Expect(ref.String()).To(Equal(tt.wantRef))
+				if tt.insecure {
+					g.Expect(ref.Context().Registry.Scheme()).To(Equal("http"))
+				}
 			}
 		})
 	}

main.go

@@ -77,6 +77,7 @@ func main() {
 		logOptions              logger.Options
 		leaderElectionOptions   leaderelection.Options
 		watchOptions            helper.WatchOptions
+		connOptions             helper.ConnectionOptions
 		storagePath             string
 		storageValueLogFileSize int64
 		concurrent              int
@@ -107,11 +108,18 @@ func main() {
 	rateLimiterOptions.BindFlags(flag.CommandLine)
 	featureGates.BindFlags(flag.CommandLine)
 	watchOptions.BindFlags(flag.CommandLine)
+	connOptions.BindFlags(flag.CommandLine)
 
 	flag.Parse()
 
 	logger.SetLogger(logger.NewLogger(logOptions))
 
+	if err := connOptions.CheckEnvironmentCompatibility(); err != nil {
+		setupLog.Error(err,
+			"please verify that your controller flag settings are compatible with the controller's environment")
+		os.Exit(1)
+	}
+
 	if awsAutoLogin || gcpAutoLogin || azureAutoLogin {
 		setupLog.Error(errors.New("use of deprecated flags"),
 			"autologin flags have been deprecated. These flags will be removed in a future release."+
@@ -216,6 +224,7 @@ func main() {
 			AzureAutoLogin: azureAutoLogin,
 			GcpAutoLogin:   gcpAutoLogin,
 		},
+		AllowInsecureHTTP: connOptions.AllowHTTP,
 	}).SetupWithManager(mgr, controller.ImageRepositoryReconcilerOptions{
 		RateLimiter: helper.GetRateLimiter(rateLimiterOptions),
 	}); err != nil {