chore: separate templates (opendatahub-io#932)

* init commit

* move const, move comment

* remove unused fixture

* Update components/kserve/feature_templates.go

Co-authored-by: Bartosz Majsak <[email protected]>

* make template structure consistent from review

* only check basepath for tmpl

* re-add template test

* make ManifestSource required to call Manifest

* fix linter?

* make review changes

* lint post rebase

* preserve prior manifest order

---------

Co-authored-by: Bartosz Majsak <[email protected]>

components/kserve/feature_resources.go

@@ -0,0 +1,31 @@
+package kserve
+
+import (
+	"embed"
+	"io/fs"
+	"path"
+)
+
+//go:embed resources
+var kserveEmbeddedFS embed.FS
+
+const baseDir = "resources"
+
+var Resources = struct {
+	// ServiceMeshDir is the path to the Service Mesh templates.
+	ServiceMeshDir string
+	// InstallDir is the path to the Serving install templates.
+	InstallDir string
+	// GatewaysDir is the path to the Serving Istio gateways templates.
+	GatewaysDir string
+	// Source the templates to be used
+	Source fs.FS
+	// BaseDir is the path to the base of the embedded FS
+	BaseDir string
+}{
+	ServiceMeshDir: path.Join(baseDir, "servicemesh"),
+	InstallDir:     path.Join(baseDir, "serving-install"),
+	GatewaysDir:    path.Join(baseDir, "servicemesh", "routing"),
+	Source:         kserveEmbeddedFS,
+	BaseDir:        baseDir,
+}

components/kserve/serverless_setup.go

@@ -12,8 +12,9 @@ func (k *Kserve) configureServerlessFeatures() feature.FeaturesProvider {
 	return func(handler *feature.FeaturesHandler) error {
 		servingDeploymentErr := feature.CreateFeature("serverless-serving-deployment").
 			For(handler).
+			ManifestSource(Resources.Source).
 			Manifests(
-				path.Join(feature.ServerlessDir, "serving-install"),
+				path.Join(Resources.InstallDir),
 			).
 			WithData(PopulateComponentSettings(k)).
 			PreConditions(
@@ -32,8 +33,9 @@ func (k *Kserve) configureServerlessFeatures() feature.FeaturesProvider {
 
 		servingNetIstioSecretFilteringErr := feature.CreateFeature("serverless-net-istio-secret-filtering").
 			For(handler).
+			ManifestSource(Resources.Source).
 			Manifests(
-				path.Join(feature.ServerlessDir, "serving-net-istio-secret-filtering.patch.tmpl"),
+				path.Join(Resources.BaseDir, "serving-net-istio-secret-filtering.patch.tmpl.yaml"),
 			).
 			WithData(PopulateComponentSettings(k)).
 			PreConditions(serverless.EnsureServerlessServingDeployed).
@@ -54,8 +56,9 @@ func (k *Kserve) configureServerlessFeatures() feature.FeaturesProvider {
 				serverless.ServingIngressDomain,
 			).
 			WithResources(serverless.ServingCertificateResource).
+			ManifestSource(Resources.Source).
 			Manifests(
-				path.Join(feature.ServerlessDir, "serving-istio-gateways"),
+				path.Join(Resources.GatewaysDir),
 			).
 			Load()
 		if serverlessGwErr != nil {

components/kserve/servicemesh_setup.go

@@ -40,11 +40,12 @@ func (k *Kserve) defineServiceMeshFeatures(cli client.Client) feature.FeaturesPr
 		if authorinoInstalled {
 			kserveExtAuthzErr := feature.CreateFeature("kserve-external-authz").
 				For(handler).
+				ManifestSource(Resources.Source).
 				Manifests(
-					path.Join(feature.KServeDir, "activator-envoyfilter.tmpl"),
-					path.Join(feature.KServeDir, "envoy-oauth-temp-fix.tmpl"),
-					path.Join(feature.KServeDir, "kserve-predictor-authorizationpolicy.tmpl"),
-					path.Join(feature.KServeDir, "z-migrations"),
+					path.Join(Resources.ServiceMeshDir, "activator-envoyfilter.tmpl.yaml"),
+					path.Join(Resources.ServiceMeshDir, "envoy-oauth-temp-fix.tmpl.yaml"),
+					path.Join(Resources.ServiceMeshDir, "kserve-predictor-authorizationpolicy.tmpl.yaml"),
+					path.Join(Resources.ServiceMeshDir, "z-migrations"),
 				).
 				WithData(servicemesh.ClusterDetails).
 				Load()
@@ -58,8 +59,9 @@ func (k *Kserve) defineServiceMeshFeatures(cli client.Client) feature.FeaturesPr
 
 		temporaryFixesErr := feature.CreateFeature("kserve-temporary-fixes").
 			For(handler).
+			ManifestSource(Resources.Source).
 			Manifests(
-				path.Join(feature.KServeDir, "grpc-envoyfilter-temp-fix.tmpl"),
+				path.Join(Resources.ServiceMeshDir, "grpc-envoyfilter-temp-fix.tmpl.yaml"),
 			).
 			WithData(servicemesh.ClusterDetails).
 			Load()

controllers/dscinitialization/feature_resources.go

@@ -0,0 +1,31 @@
+package dscinitialization
+
+import (
+	"embed"
+	"io/fs"
+	"path"
+)
+
+//go:embed resources
+var dsciEmbeddedFS embed.FS
+
+const baseDir = "resources"
+
+var Templates = struct {
+	// ServiceMeshDir is the path to the Service Mesh templates.
+	ServiceMeshDir string
+	// InstallDir is the path to the Serving install templates.
+	AuthorinoDir string
+	// GatewaysDir is the path to the Serving Istio gateways templates.
+	MetricsDir string
+	// Source the templates to be used
+	Source fs.FS
+	// BaseDir is the path to the base of the embedded FS
+	BaseDir string
+}{
+	ServiceMeshDir: path.Join(baseDir, "servicemesh"),
+	AuthorinoDir:   path.Join(baseDir, "authorino"),
+	MetricsDir:     path.Join(baseDir, "metrics-collection"),
+	Source:         dsciEmbeddedFS,
+	BaseDir:        baseDir,
+}

controllers/dscinitialization/servicemesh_setup.go

@@ -118,8 +118,9 @@ func (r *DSCInitializationReconciler) serviceMeshCapabilityFeatures(instance *ds
 		serviceMeshSpec := instance.Spec.ServiceMesh
 		smcpCreationErr := feature.CreateFeature("mesh-control-plane-creation").
 			For(handler).
+			ManifestSource(Templates.Source).
 			Manifests(
-				path.Join(feature.ServiceMeshDir, "base", "create-smcp.tmpl"),
+				path.Join(Templates.ServiceMeshDir),
 			).
 			PreConditions(
 				servicemesh.EnsureServiceMeshOperatorInstalled,
@@ -140,8 +141,9 @@ func (r *DSCInitializationReconciler) serviceMeshCapabilityFeatures(instance *ds
 				PreConditions(
 					servicemesh.EnsureServiceMeshInstalled,
 				).
+				ManifestSource(Templates.Source).
 				Manifests(
-					path.Join(feature.ServiceMeshDir, "metrics-collection"),
+					path.Join(Templates.MetricsDir),
 				).
 				Load()
 			if metricsCollectionErr != nil {
@@ -167,10 +169,11 @@ func (r *DSCInitializationReconciler) authorizationFeatures(instance *dsciv1.DSC
 
 		extAuthzErr := feature.CreateFeature("mesh-control-plane-external-authz").
 			For(handler).
+			ManifestSource(Templates.Source).
 			Manifests(
-				path.Join(feature.AuthDir, "auth-smm.tmpl"),
-				path.Join(feature.AuthDir, "base"),
-				path.Join(feature.AuthDir, "mesh-authz-ext-provider.patch.tmpl"),
+				path.Join(Templates.AuthorinoDir, "auth-smm.tmpl.yaml"),
+				path.Join(Templates.AuthorinoDir, "base"),
+				path.Join(Templates.AuthorinoDir, "mesh-authz-ext-provider.patch.tmpl.yaml"),
 			).
 			WithData(servicemesh.ClusterDetails).
 			PreConditions(
@@ -189,7 +192,7 @@ func (r *DSCInitializationReconciler) authorizationFeatures(instance *dsciv1.DSC
 					//
 					// To make it part of Service Mesh we have to patch it with injection
 					// enabled instead, otherwise it will not have proxy pod injected.
-					return f.ApplyManifest(path.Join(feature.AuthDir, "deployment.injection.patch.tmpl"))
+					return f.ApplyManifest(path.Join(Templates.AuthorinoDir, "deployment.injection.patch.tmpl.yaml"))
 				},
 			).
 			OnDelete(

pkg/feature/builder.go

@@ -53,7 +53,6 @@ func (u *usingFeaturesHandler) For(featuresHandler *FeaturesHandler) *featureBui
 	fb := &featureBuilder{
 		name:            u.name,
 		featuresHandler: featuresHandler,
-		fsys:            embeddedFiles,
 		targetNS:        featuresHandler.DSCInitializationSpec.ApplicationsNamespace,
 	}
 
@@ -85,7 +84,18 @@ func createClient(config *rest.Config) partialBuilder {
 	}
 }
 
-func (fb *featureBuilder) Manifests(paths ...string) *featureBuilder {
+// Used to enforce that Manifests() is called after ManifestSource() in the chain.
+type featureBuilderWithManifestSource struct {
+	*featureBuilder
+}
+
+// ManifestSource sets the root file system (fsys) from which manifest paths are loaded.
+func (fb *featureBuilder) ManifestSource(fsys fs.FS) *featureBuilderWithManifestSource {
+	fb.fsys = fsys
+	return &featureBuilderWithManifestSource{featureBuilder: fb}
+}
+
+func (fb *featureBuilderWithManifestSource) Manifests(paths ...string) *featureBuilderWithManifestSource {
 	fb.builders = append(fb.builders, func(f *Feature) error {
 		var err error
 		var manifests []Manifest
@@ -177,6 +187,7 @@ func (fb *featureBuilder) Load() error {
 	}
 
 	feature.Spec.TargetNamespace = fb.targetNS
+	feature.fsys = fb.fsys
 
 	fb.featuresHandler.features = append(fb.featuresHandler.features, feature)
 
@@ -204,13 +215,6 @@ func (fb *featureBuilder) withDefaultClient() error {
 	return nil
 }
 
-// ManifestSource sets the root file system (fsys) from which manifest paths are loaded
-// If ManifestSource is not called in the builder chain, the default source will be the embeddedFiles.
-func (fb *featureBuilder) ManifestSource(fsys fs.FS) *featureBuilder {
-	fb.fsys = fsys
-	return fb
-}
-
 func (fb *featureBuilder) TargetNamespace(targetNs string) *featureBuilder {
 	fb.targetNS = targetNs
 

pkg/feature/feature.go

@@ -3,6 +3,7 @@ package feature
 import (
 	"context"
 	"fmt"
+	"io/fs"
 
 	"github.com/go-logr/logr"
 	"github.com/hashicorp/go-multierror"
@@ -33,6 +34,7 @@ type Feature struct {
 	preconditions  []Action
 	postconditions []Action
 	loaders        []Action
+	fsys           fs.FS
 
 	Log logr.Logger
 }
@@ -169,7 +171,7 @@ func (f *Feature) addCleanup(cleanupFuncs ...Action) {
 }
 
 func (f *Feature) ApplyManifest(path string) error {
-	m, err := loadManifestsFrom(embeddedFiles, path)
+	m, err := loadManifestsFrom(f.fsys, path)
 	if err != nil {
 		return err
 	}

pkg/feature/manifest.go

@@ -2,12 +2,10 @@ package feature
 
 import (
 	"bytes"
-	"embed"
 	"fmt"
 	"html/template"
 	"io"
 	"io/fs"
-	"path"
 	"path/filepath"
 	"regexp"
 	"strings"
@@ -16,17 +14,6 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 )
 
-//go:embed templates
-var embeddedFiles embed.FS
-
-var (
-	BaseDir        = "templates"
-	ServiceMeshDir = path.Join(BaseDir, "servicemesh")
-	ServerlessDir  = path.Join(BaseDir, "serverless")
-	AuthDir        = path.Join(ServiceMeshDir, "authorino")
-	KServeDir      = path.Join(ServiceMeshDir, "kserve")
-)
-
 type Manifest interface {
 	// Process allows any arbitrary struct to be passed and used while processing the content of the manifest.
 	Process(data any) ([]*unstructured.Unstructured, error)
@@ -145,13 +132,13 @@ func CreateTemplateManifestFrom(fsys fs.FS, path string) *templateManifest { //n
 	return &templateManifest{
 		name:  basePath,
 		path:  path,
-		patch: strings.Contains(basePath, ".patch"),
+		patch: strings.Contains(basePath, ".patch."),
 		fsys:  fsys,
 	}
 }
 
 func isTemplateManifest(path string) bool {
-	return strings.Contains(path, ".tmpl")
+	return strings.Contains(filepath.Base(path), ".tmpl.")
 }
 
 func convertToUnstructuredSlice(resources string) ([]*unstructured.Unstructured, error) {

tests/integration/features/fixtures/const.go

@@ -7,4 +7,5 @@ const (
 	Timeout = 5 * time.Second
 	// Interval is the default interval for polling for a condition to be met.
 	Interval = 250 * time.Millisecond
+	BaseDir  = "templates"
 )

tests/integration/features/fixtures/templates/local-gateway-svc.tmpl.yaml

@@ -0,0 +1,16 @@
+apiVersion: v1
+kind: Service
+metadata:
+  labels:
+    experimental.istio.io/disable-gateway-port-translation: "true"
+  name: knative-local-gateway
+  namespace: {{ .ControlPlane.Namespace }}
+spec:
+  ports:
+    - name: http2
+      port: 80
+      protocol: TCP
+      targetPort: 8081
+  selector:
+    knative: ingressgateway
+  type: ClusterIP

tests/integration/features/fixtures/templates/mesh-authz-ext-provider.patch.tmpl.yaml

@@ -0,0 +1,13 @@
+apiVersion: maistra.io/v2
+kind: ServiceMeshControlPlane
+metadata:
+  name: {{ .ControlPlane.Name }}
+  namespace: {{ .ControlPlane.Namespace }}
+spec:
+  techPreview:
+    meshConfig:
+      extensionProviders:
+      - name: {{ .AppNamespace }}-auth-provider
+        envoyExtAuthzGrpc:
+          service: {{ .AuthProviderName }}-authorino-authorization.{{ .Auth.Namespace }}.svc.cluster.local
+          port: 50051