reverseproxy: Pointer to struct when loading modules; remove LazyCertPool (#6307)

* use pointer when loading modules

* change method to pointer type and remove LazyCertPool

* remove lazy pool test

* remove yet another lazy pool test

modules/caddyhttp/reverseproxy/httptransport.go

@@ -541,7 +541,7 @@ type TLSConfig struct {
 
 // MakeTLSClientConfig returns a tls.Config usable by a client to a backend.
 // If there is no custom TLS configuration, a nil config may be returned.
-func (t TLSConfig) MakeTLSClientConfig(ctx caddy.Context) (*tls.Config, error) {
+func (t *TLSConfig) MakeTLSClientConfig(ctx caddy.Context) (*tls.Config, error) {
 	cfg := new(tls.Config)
 
 	// client auth

modules/caddytls/capools.go

@@ -27,7 +27,6 @@ func init() {
 	caddy.RegisterModule(PKIIntermediateCAPool{})
 	caddy.RegisterModule(StoragePool{})
 	caddy.RegisterModule(HTTPCertPool{})
-	caddy.RegisterModule(LazyCertPool{})
 }
 
 // The interface to be implemented by all guest modules part of
@@ -500,7 +499,7 @@ func (t *TLSConfig) unmarshalCaddyfile(d *caddyfile.Dispenser) error {
 // MakeTLSClientConfig returns a tls.Config usable by a client to a backend.
 // If there is no custom TLS configuration, a nil config may be returned.
 // copied from with minor modifications: modules/caddyhttp/reverseproxy/httptransport.go
-func (t TLSConfig) makeTLSClientConfig(ctx caddy.Context) (*tls.Config, error) {
+func (t *TLSConfig) makeTLSClientConfig(ctx caddy.Context) (*tls.Config, error) {
 	repl := ctx.Value(caddy.ReplacerCtxKey).(*caddy.Replacer)
 	if repl == nil {
 		repl = caddy.NewReplacer()
@@ -664,121 +663,6 @@ func (hcp HTTPCertPool) CertPool() *x509.CertPool {
 	return hcp.pool
 }
 
-// LazyCertPool defers the generation of the certificate pool from the
-// guest module to demand-time rather than at provisionig time. The gain of the
-// lazy load adds a risk of failure to load the certificates at demand time
-// because the validation that's typically done at provisioning is deferred.
-// The validation can be enforced to run before runtime by setting
-// `EagerValidation`/`eager_validation` to `true`. It is the operator's responsibility
-// to ensure the resources are available if `EagerValidation`/`eager_validation`
-// is set to `true`. The module also incurs performance cost at every demand.
-type LazyCertPool struct {
-	// Provides the guest module that provides the trusted certificate authority (CA) certificates
-	CARaw json.RawMessage `json:"ca,omitempty" caddy:"namespace=tls.ca_pool.source inline_key=provider"`
-
-	// Whether the validation step should try to load and provision the guest module to validate
-	// the correctness of the configuration. Depeneding on the type of the guest module,
-	// the resources may not be available at validation time. It is the
-	// operator's responsibility to ensure the resources are available if `EagerValidation`/`eager_validation`
-	// is set to `true`.
-	EagerValidation bool `json:"eager_validation,omitempty"`
-
-	ctx caddy.Context
-}
-
-// CaddyModule implements caddy.Module.
-func (LazyCertPool) CaddyModule() caddy.ModuleInfo {
-	return caddy.ModuleInfo{
-		ID: "tls.ca_pool.source.lazy",
-		New: func() caddy.Module {
-			return new(LazyCertPool)
-		},
-	}
-}
-
-// Provision implements caddy.Provisioner.
-func (lcp *LazyCertPool) Provision(ctx caddy.Context) error {
-	if len(lcp.CARaw) == 0 {
-		return fmt.Errorf("missing backing CA source")
-	}
-	lcp.ctx = ctx
-	return nil
-}
-
-// Syntax:
-//
-//	trust_pool lazy {
-//		backend <ca_module>
-//		eager_validation
-//	}
-//
-// The `backend` directive specifies the CA module to use to provision the
-// certificate pool. The `eager_validation` directive specifies that the
-// validation step should try to load and provision the guest module to validate
-// the correctness of the configuration. Depeneding on the type of the guest module,
-// the resources may not be available at validation time. It is the
-// operator's responsibility to ensure the resources are available if `EagerValidation`/`eager_validation`
-// is set to `true`.
-//
-// The `backend` directive is required.
-func (lcp *LazyCertPool) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
-	d.Next() // consume module name
-	for nesting := d.Nesting(); d.NextBlock(nesting); {
-		switch d.Val() {
-		case "backend":
-			if lcp.CARaw != nil {
-				return d.Err("backend block already defined")
-			}
-			if !d.NextArg() {
-				return d.ArgErr()
-			}
-			modStem := d.Val()
-			modID := "tls.ca_pool.source." + modStem
-			unm, err := caddyfile.UnmarshalModule(d, modID)
-			if err != nil {
-				return err
-			}
-			backend, ok := unm.(CA)
-			if !ok {
-				return d.Errf("module %s is not a caddytls.CA", modID)
-			}
-			lcp.CARaw = caddyconfig.JSONModuleObject(backend, "provider", modStem, nil)
-		case "eager_validation":
-			lcp.EagerValidation = true
-		default:
-			return d.Errf("unrecognized directive: %s", d.Val())
-		}
-	}
-	if lcp.CARaw == nil {
-		return d.Err("backend block is required")
-	}
-	return nil
-}
-
-// If EagerValidation is `true`, it attempts to load and provision the guest module
-// to ensure the guesst module's configuration is correct. Depeneding on the type of the
-// guest module, the resources may not be available at validation time. It is the
-// operator's responsibility to ensure the resources are available if `EagerValidation` is
-// set to `true`.
-func (lcp LazyCertPool) Validate() error {
-	if lcp.EagerValidation {
-		_, err := lcp.ctx.LoadModule(lcp, "CARaw")
-		return err
-	}
-	return nil
-}
-
-// CertPool loads the guest module and returns the CertPool from there
-// TODO: Cache?
-func (lcp LazyCertPool) CertPool() *x509.CertPool {
-	caRaw, err := lcp.ctx.LoadModule(lcp, "CARaw")
-	if err != nil {
-		return nil
-	}
-	ca := caRaw.(CA)
-	return ca.CertPool()
-}
-
 var (
 	_ caddy.Module          = (*InlineCAPool)(nil)
 	_ caddy.Provisioner     = (*InlineCAPool)(nil)
@@ -810,10 +694,4 @@ var (
 	_ caddy.Validator       = (*HTTPCertPool)(nil)
 	_ CA                    = (*HTTPCertPool)(nil)
 	_ caddyfile.Unmarshaler = (*HTTPCertPool)(nil)
-
-	_ caddy.Module          = (*LazyCertPool)(nil)
-	_ caddy.Provisioner     = (*LazyCertPool)(nil)
-	_ caddy.Validator       = (*LazyCertPool)(nil)
-	_ CA                    = (*LazyCertPool)(nil)
-	_ caddyfile.Unmarshaler = (*LazyCertPool)(nil)
 )

modules/caddytls/capools_test.go

@@ -566,21 +566,6 @@ func TestTLSConfig_unmarshalCaddyfile(t *testing.T) {
 				CARaw: []byte(`{"pem_files":["/var/caddy/ca.pem"],"provider":"file"}`),
 			},
 		},
-		{
-			name: "setting 'ca' to 'lazy' with appropriate block is valid",
-			args: args{
-				d: caddyfile.NewTestDispenser(`{
-					ca lazy {
-						backend file {
-							pem_file /var/caddy/ca.pem
-						}
-					}
-				}`),
-			},
-			expected: TLSConfig{
-				CARaw: []byte(`{"ca":{"pem_files":["/var/caddy/ca.pem"],"provider":"file"},"provider":"lazy"}`),
-			},
-		},
 		{
 			name: "setting 'ca' to 'file' with appropriate block is valid",
 			args: args{
@@ -791,102 +776,3 @@ func TestHTTPCertPoolUnmarshalCaddyfile(t *testing.T) {
 		})
 	}
 }
-
-func TestLazyCertPoolUnmarshalCaddyfile(t *testing.T) {
-	type args struct {
-		d *caddyfile.Dispenser
-	}
-	tests := []struct {
-		name     string
-		args     args
-		expected LazyCertPool
-		wantErr  bool
-	}{
-		{
-			name: "no block results in error",
-			args: args{
-				d: caddyfile.NewTestDispenser(`lazy`),
-			},
-			wantErr: true,
-		},
-		{
-			name: "empty block results in error",
-			args: args{
-				d: caddyfile.NewTestDispenser(`lazy {
-				}`),
-			},
-			wantErr: true,
-		},
-		{
-			name: "defining 'backend' multiple times results in error",
-			args: args{
-				d: caddyfile.NewTestDispenser(`lazy {
-					backend http {
-						endpoints http://localhost/ca-certs
-					}
-					backend file {
-						pem_file /var/caddy/certs
-					}
-				}`),
-			},
-			wantErr: true,
-		},
-		{
-			name: "defining 'backend' without argument results in error",
-			args: args{
-				d: caddyfile.NewTestDispenser(`lazy {
-					backend
-				}`),
-			},
-			wantErr: true,
-		},
-		{
-			name: "using unrecognized directive results in error",
-			args: args{
-				d: caddyfile.NewTestDispenser(`lazy {
-					foo
-				}`),
-			},
-			wantErr: true,
-		},
-		{
-			name: "defining single 'backend' is successful",
-			args: args{
-				d: caddyfile.NewTestDispenser(`lazy {
-					backend http {
-						endpoints http://localhost/ca-certs
-					}
-				}`),
-			},
-			expected: LazyCertPool{
-				CARaw: []byte(`{"endpoints":["http://localhost/ca-certs"],"provider":"http"}`),
-			},
-		},
-		{
-			name: "defining single 'backend' with 'eager_validation' successful",
-			args: args{
-				d: caddyfile.NewTestDispenser(`lazy {
-					backend file {
-						pem_file /var/caddy/certs
-					}
-					eager_validation
-				}`),
-			},
-			expected: LazyCertPool{
-				CARaw:           []byte(`{"pem_files":["/var/caddy/certs"],"provider":"file"}`),
-				EagerValidation: true,
-			},
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			lcp := &LazyCertPool{}
-			if err := lcp.UnmarshalCaddyfile(tt.args.d); (err != nil) != tt.wantErr {
-				t.Errorf("LazyCertPool.UnmarshalCaddyfile() error = %v, wantErr %v", err, tt.wantErr)
-			}
-			if !tt.wantErr && !reflect.DeepEqual(&tt.expected, lcp) {
-				t.Errorf("LazyCertPool.UnmarshalCaddyfile() = %v, want %v", lcp, tt.expected)
-			}
-		})
-	}
-}