ref(*): move feature flags to meshconfig

Move feature flag specification from the osm deployment to the meshconfig. Allows the feature flags to be updated without recreating the mesh. Signed-off-by: Jackie Elliott <[email protected]>
openservicemesh · Jun 9, 2021 · 55e3b3b · 55e3b3b
1 parent 15cce1d
commit 55e3b3b
Show file tree

Hide file tree

Showing 32 changed files with 242 additions and 194 deletions.
diff --git a/charts/osm/crds/meshconfig.yaml b/charts/osm/crds/meshconfig.yaml
@@ -180,3 +180,16 @@ spec:
                       description: Sets the service certificate validity duration, represented as a sequence of decimal numbers each with optional fraction and a unit suffix.
                       type: string
                       default: "24h"
+                featureFlags:
+                  description: OSM feature flags
+                  type: object
+                  properties:
+                    enableWASMStats:
+                      type: boolean
+                      default: true
+                    enableEgressPolicy:
+                      type: boolean
+                      default: true
+                    enableMulticlusterMode:
+                      type: boolean
+                      default: false
diff --git a/charts/osm/templates/osm-deployment.yaml b/charts/osm/templates/osm-deployment.yaml
@@ -67,15 +67,6 @@ spec:
             "--cert-manager-issuer-name", "{{.Values.OpenServiceMesh.certmanager.issuerName}}",
             "--cert-manager-issuer-kind", "{{.Values.OpenServiceMesh.certmanager.issuerKind}}",
             "--cert-manager-issuer-group", "{{.Values.OpenServiceMesh.certmanager.issuerGroup}}",
-            {{- if .Values.OpenServiceMesh.featureFlags.enableWASMStats }}
-            "--stats-wasm-experimental",
-            {{- end }}
-            {{- if .Values.OpenServiceMesh.featureFlags.enableEgressPolicy }}
-            "--enable-egress-policy",
-            {{- end }}
-            {{- if .Values.OpenServiceMesh.featureFlags.enableMulticlusterMode }}
-            "--enable-multicluster",
-            {{- end }}
           ]
           resources:
             limits:

diff --git a/charts/osm/templates/preset-mesh-config.yaml b/charts/osm/templates/preset-mesh-config.yaml
@@ -26,4 +26,8 @@ spec:
       endpoint: {{.Values.OpenServiceMesh.tracing.endpoint  | quote}}
       {{- end }}
   certificate:
-    serviceCertValidityDuration: {{.Values.OpenServiceMesh.serviceCertValidityDuration}}
+    serviceCertValidityDuration: {{.Values.OpenServiceMesh.serviceCertValidityDuration}}
+  featureFlags: 
+    enableWASMStats: {{.Values.OpenServiceMesh.featureFlags.enableWASMStats}}
+    enableEgressPolicy: {{.Values.OpenServiceMesh.featureFlags.enableEgressPolicy}}
+    enableMulticlusterMode: {{.Values.OpenServiceMesh.featureFlags.enableMulticlusterMode}}
diff --git a/cmd/osm-controller/osm-controller.go b/cmd/osm-controller/osm-controller.go
@@ -5,7 +5,6 @@ package main
 
 import (
 	"context"
-	"encoding/json"
 	"flag"
 	"fmt"
 	"net/http"
@@ -34,7 +33,6 @@ import (
 	"github.com/openservicemesh/osm/pkg/endpoint/providers/kube"
 	"github.com/openservicemesh/osm/pkg/envoy/ads"
 	"github.com/openservicemesh/osm/pkg/envoy/registry"
-	"github.com/openservicemesh/osm/pkg/featureflags"
 	"github.com/openservicemesh/osm/pkg/gen/client/config/clientset/versioned"
 	"github.com/openservicemesh/osm/pkg/health"
 	"github.com/openservicemesh/osm/pkg/httpserver"
@@ -68,9 +66,6 @@ var (
 	vaultOptions       providers.VaultOptions
 	certManagerOptions providers.CertManagerOptions
 
-	// feature flag options
-	optionalFeatures featureflags.OptionalFeatures
-
 	scheme = runtime.NewScheme()
 )
 
@@ -103,11 +98,6 @@ func init() {
 	flags.StringVar(&certManagerOptions.IssuerKind, "cert-manager-issuer-kind", "Issuer", "cert-manager issuer kind")
 	flags.StringVar(&certManagerOptions.IssuerGroup, "cert-manager-issuer-group", "cert-manager.io", "cert-manager issuer group")
 
-	// feature flags
-	flags.BoolVar(&optionalFeatures.WASMStats, "stats-wasm-experimental", false, "Enable a WebAssembly module that generates additional Envoy statistics")
-	flags.BoolVar(&optionalFeatures.EgressPolicy, "enable-egress-policy", false, "Enable OSM's Egress policy API")
-	flags.BoolVar(&optionalFeatures.MulticlusterMode, "enable-multicluster", false, "Enable multicluster mode in OSM")
-
 	_ = clientgoscheme.AddToScheme(scheme)
 	_ = admissionv1.AddToScheme(scheme)
 }
@@ -122,13 +112,6 @@ func main() {
 		log.Fatal().Err(err).Msg("Error setting log level")
 	}
 
-	if featureFlagsJSON, err := json.Marshal(featureflags.Features); err != nil {
-		log.Error().Err(err).Msgf("Error marshaling feature flags struct: %+v", featureflags.Features)
-	} else {
-		log.Info().Msgf("Feature flags: %s", string(featureFlagsJSON))
-	}
-
-	featureflags.Initialize(optionalFeatures)
 	events.GetPubSubInstance() // Just to generate the interface, single routine context
 
 	// Initialize kube config and client

diff --git a/pkg/apis/config/v1alpha1/mesh_config.go b/pkg/apis/config/v1alpha1/mesh_config.go
@@ -35,6 +35,9 @@ type MeshConfigSpec struct {
 
 	// Certificate defines the certificate management configurations for a mesh instance.
 	Certificate CertificateSpec `json:"certificate,omitempty"`
+
+	// FeatureFlags defines the feature flags for a mesh instance.
+	FeatureFlags FeatureFlags `json:"featureFlags,omitempty"`
 }
 
 // SidecarSpec is the type used to represent the specifications for the proxy sidecar.
@@ -144,3 +147,15 @@ type MeshConfigList struct {
 
 	Items []MeshConfig `json:"items"`
 }
+
+// FeatureFlags is a type to represent OSM's feature flags.
+type FeatureFlags struct {
+	// EnableWASMStats defines if WASM Stats are enabled.
+	EnableWASMStats bool `json:"enableWASMStats,omitempty"`
+
+	// EnableEgressPolicy defines if OSM's Egress policy is enabled.
+	EnableEgressPolicy bool `json:"enableEgressPolicy,omitempty"`
+
+	// EnableMulticlusterMode defines if Multicluster mode is enabled.
+	EnableMulticlusterMode bool `json:"enableMulticlusterMode,omitempty"`
+}
diff --git a/pkg/catalog/egress.go b/pkg/catalog/egress.go
@@ -11,15 +11,14 @@ import (
 	policyV1alpha1 "github.com/openservicemesh/osm/pkg/apis/policy/v1alpha1"
 
 	"github.com/openservicemesh/osm/pkg/constants"
-	"github.com/openservicemesh/osm/pkg/featureflags"
 	"github.com/openservicemesh/osm/pkg/identity"
 	"github.com/openservicemesh/osm/pkg/service"
 	"github.com/openservicemesh/osm/pkg/trafficpolicy"
 )
 
 // GetEgressTrafficPolicy returns the Egress traffic policy associated with the given service identity
 func (mc *MeshCatalog) GetEgressTrafficPolicy(serviceIdentity identity.ServiceIdentity) (*trafficpolicy.EgressTrafficPolicy, error) {
-	if !featureflags.IsEgressPolicyEnabled() {
+	if !mc.configurator.GetFeatureFlags().EnableEgressPolicy {
 		return nil, nil
 	}
 

diff --git a/pkg/catalog/egress_test.go b/pkg/catalog/egress_test.go
@@ -13,26 +13,22 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/utils/pointer"
 
+	"github.com/openservicemesh/osm/pkg/apis/config/v1alpha1"
 	policyV1alpha1 "github.com/openservicemesh/osm/pkg/apis/policy/v1alpha1"
+	"github.com/openservicemesh/osm/pkg/configurator"
 	"github.com/openservicemesh/osm/pkg/identity"
 	"github.com/openservicemesh/osm/pkg/policy"
 
-	"github.com/openservicemesh/osm/pkg/featureflags"
 	"github.com/openservicemesh/osm/pkg/service"
 	"github.com/openservicemesh/osm/pkg/smi"
 	"github.com/openservicemesh/osm/pkg/trafficpolicy"
 )
 
-func init() {
-	optionalFeatures := featureflags.OptionalFeatures{
-		EgressPolicy: true,
-	}
-	featureflags.Initialize(optionalFeatures)
-}
-
 func TestGetEgressTrafficPolicy(t *testing.T) {
 	assert := tassert.New(t)
 	mockCtrl := gomock.NewController(t)
+	mockCfg := configurator.NewMockConfigurator(mockCtrl)
+
 	defer mockCtrl.Finish()
 
 	testCases := []struct {
@@ -354,9 +350,12 @@ func TestGetEgressTrafficPolicy(t *testing.T) {
 
 			mc := &MeshCatalog{
 				meshSpec:         mockMeshSpec,
+				configurator:     mockCfg,
 				policyController: mockPolicyController,
 			}
 
+			mockCfg.EXPECT().GetFeatureFlags().Return(v1alpha1.FeatureFlags{EnableEgressPolicy: true}).Times(1)
+
 			actual, err := mc.GetEgressTrafficPolicy(testSourceIdentity)
 			assert.Equal(tc.expectError, err != nil)
 			assert.ElementsMatch(tc.expectedEgressPolicy.TrafficMatches, actual.TrafficMatches)

diff --git a/pkg/catalog/types.go b/pkg/catalog/types.go
@@ -91,7 +91,7 @@ type MeshCataloger interface {
 	// ListMeshServicesForIdentity lists the services for a given service identity.
 	ListMeshServicesForIdentity(identity.ServiceIdentity) []service.MeshService
 
-	// GetEgressTrafficPolicy returns the Egress traffic policy associated with the given service identity
+	// GetEgressTrafficPolicy returns the Egress traffic policy associated with the given service identity.
 	GetEgressTrafficPolicy(identity.ServiceIdentity) (*trafficpolicy.EgressTrafficPolicy, error)
 
 	// GetKubeController returns the kube controller instance handling the current cluster

diff --git a/pkg/configurator/methods.go b/pkg/configurator/methods.go
@@ -193,3 +193,8 @@ func (c *Client) GetInboundExternalAuthConfig() auth.ExtAuthConfig {
 
 	return extAuthConfig
 }
+
+// GetFeatureFlags returns OSM's feature flags
+func (c *Client) GetFeatureFlags() v1alpha1.FeatureFlags {
+	return c.getMeshConfig().Spec.FeatureFlags
+}
diff --git a/pkg/configurator/methods_test.go b/pkg/configurator/methods_test.go
@@ -404,6 +404,51 @@ func TestCreateUpdateConfig(t *testing.T) {
 				assert.Equal(resource.MustParse("512M"), res.Limits[v1.ResourceMemory])
 			},
 		},
+		{
+			name:                  "IsWASMStatsEnabled",
+			initialMeshConfigData: &v1alpha1.MeshConfigSpec{},
+			checkCreate: func(assert *tassert.Assertions, cfg Configurator) {
+				assert.Equal(false, cfg.GetFeatureFlags().EnableWASMStats)
+			},
+			updatedMeshConfigData: &v1alpha1.MeshConfigSpec{
+				FeatureFlags: v1alpha1.FeatureFlags{
+					EnableWASMStats: true,
+				},
+			},
+			checkUpdate: func(assert *tassert.Assertions, cfg Configurator) {
+				assert.Equal(true, cfg.GetFeatureFlags().EnableWASMStats)
+			},
+		},
+		{
+			name:                  "IsEgressPolicyEnabled",
+			initialMeshConfigData: &v1alpha1.MeshConfigSpec{},
+			checkCreate: func(assert *tassert.Assertions, cfg Configurator) {
+				assert.Equal(false, cfg.GetFeatureFlags().EnableEgressPolicy)
+			},
+			updatedMeshConfigData: &v1alpha1.MeshConfigSpec{
+				FeatureFlags: v1alpha1.FeatureFlags{
+					EnableEgressPolicy: true,
+				},
+			},
+			checkUpdate: func(assert *tassert.Assertions, cfg Configurator) {
+				assert.Equal(true, cfg.GetFeatureFlags().EnableEgressPolicy)
+			},
+		},
+		{
+			name:                  "IsMulticlusterModeEnabled",
+			initialMeshConfigData: &v1alpha1.MeshConfigSpec{},
+			checkCreate: func(assert *tassert.Assertions, cfg Configurator) {
+				assert.Equal(false, cfg.GetFeatureFlags().EnableMulticlusterMode)
+			},
+			updatedMeshConfigData: &v1alpha1.MeshConfigSpec{
+				FeatureFlags: v1alpha1.FeatureFlags{
+					EnableMulticlusterMode: true,
+				},
+			},
+			checkUpdate: func(assert *tassert.Assertions, cfg Configurator) {
+				assert.Equal(true, cfg.GetFeatureFlags().EnableMulticlusterMode)
+			},
+		},
 	}
 
 	for _, test := range tests {

diff --git a/pkg/configurator/mock_client_generated.go b/pkg/configurator/mock_client_generated.go
diff --git a/pkg/configurator/types.go b/pkg/configurator/types.go
@@ -7,6 +7,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/client-go/tools/cache"
 
+	"github.com/openservicemesh/osm/pkg/apis/config/v1alpha1"
 	"github.com/openservicemesh/osm/pkg/auth"
 	"github.com/openservicemesh/osm/pkg/logger"
 )
@@ -90,4 +91,7 @@ type Configurator interface {
 
 	// GetInboundExternalAuthConfig returns the External Authentication configuration for incoming traffic, if any
 	GetInboundExternalAuthConfig() auth.ExtAuthConfig
+
+	// GetFeatureFlags returns OSM's feature flags
+	GetFeatureFlags() v1alpha1.FeatureFlags
 }
diff --git a/pkg/debugger/feature_flags.go b/pkg/debugger/feature_flags.go
@@ -4,14 +4,13 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
-
-	"github.com/openservicemesh/osm/pkg/featureflags"
 )
 
 func (ds DebugConfig) getFeatureFlags() http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		if featureFlagsJSON, err := json.Marshal(featureflags.Features); err != nil {
-			log.Error().Err(err).Msgf("Error marshaling feature flags struct: %+v", featureflags.Features)
+		featureFlags := ds.configurator.GetFeatureFlags()
+		if featureFlagsJSON, err := json.Marshal(featureFlags); err != nil {
+			log.Error().Err(err).Msgf("Error marshaling feature flags struct: %+v", featureFlags)
 		} else {
 			_, _ = fmt.Fprint(w, string(featureFlagsJSON))
 		}

diff --git a/pkg/envoy/ads/response_test.go b/pkg/envoy/ads/response_test.go
@@ -17,6 +17,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	testclient "k8s.io/client-go/kubernetes/fake"
 
+	"github.com/openservicemesh/osm/pkg/apis/config/v1alpha1"
 	"github.com/openservicemesh/osm/pkg/auth"
 	configFake "github.com/openservicemesh/osm/pkg/gen/client/config/clientset/versioned/fake"
 	"github.com/openservicemesh/osm/pkg/kubernetes"
@@ -135,6 +136,10 @@ var _ = Describe("Test ADS response functions", func() {
 		mockConfigurator.EXPECT().IsPermissiveTrafficPolicyMode().Return(false).AnyTimes()
 		mockConfigurator.EXPECT().GetServiceCertValidityPeriod().Return(certDuration).AnyTimes()
 		mockConfigurator.EXPECT().IsDebugServerEnabled().Return(true).AnyTimes()
+		mockConfigurator.EXPECT().GetFeatureFlags().Return(v1alpha1.FeatureFlags{
+			EnableWASMStats:    false,
+			EnableEgressPolicy: false,
+		}).AnyTimes()
 
 		It("returns Aggregated Discovery Service response", func() {
 			s := NewADSServer(mc, proxyRegistry, true, tests.Namespace, mockConfigurator, mockCertManager, kubectrlMock)

diff --git a/pkg/envoy/cds/response_test.go b/pkg/envoy/cds/response_test.go
@@ -420,11 +420,12 @@ func TestNewResponseGetEgressTrafficPolicyError(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	meshCatalog := catalog.NewMockMeshCataloger(ctrl)
 	mockKubeController := k8s.NewMockController(ctrl)
+	cfg := configurator.NewMockConfigurator(ctrl)
+
 	meshCatalog.EXPECT().ListAllowedOutboundServicesForIdentity(proxyIdentity).Return(nil).Times(1)
 	meshCatalog.EXPECT().GetEgressTrafficPolicy(proxyIdentity).Return(nil, errors.New("some error")).Times(1)
 	meshCatalog.EXPECT().GetKubeController().Return(mockKubeController).AnyTimes()
 	mockKubeController.EXPECT().ListPods().Return([]*v1.Pod{})
-	cfg := configurator.NewMockConfigurator(ctrl)
 	cfg.EXPECT().IsEgressEnabled().Return(false).Times(1)
 	cfg.EXPECT().IsTracingEnabled().Return(false).Times(1)
 
@@ -445,6 +446,7 @@ func TestNewResponseGetEgressTrafficPolicyNotEmpty(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	meshCatalog := catalog.NewMockMeshCataloger(ctrl)
 	mockKubeController := k8s.NewMockController(ctrl)
+	cfg := configurator.NewMockConfigurator(ctrl)
 	meshCatalog.EXPECT().ListAllowedOutboundServicesForIdentity(proxyIdentity).Return(nil).Times(1)
 	meshCatalog.EXPECT().GetKubeController().Return(mockKubeController).AnyTimes()
 	mockKubeController.EXPECT().ListPods().Return([]*v1.Pod{})
@@ -454,7 +456,6 @@ func TestNewResponseGetEgressTrafficPolicyNotEmpty(t *testing.T) {
 			{Name: "my-cluster"}, // the test ensures this duplicate is removed
 		},
 	}, nil).Times(1)
-	cfg := configurator.NewMockConfigurator(ctrl)
 	cfg.EXPECT().IsEgressEnabled().Return(false).Times(1)
 	cfg.EXPECT().IsTracingEnabled().Return(false).Times(1)
 

diff --git a/pkg/envoy/lds/connection_manager.go b/pkg/envoy/lds/connection_manager.go
@@ -14,7 +14,6 @@ import (
 	"github.com/openservicemesh/osm/pkg/configurator"
 	"github.com/openservicemesh/osm/pkg/constants"
 	"github.com/openservicemesh/osm/pkg/envoy"
-	"github.com/openservicemesh/osm/pkg/featureflags"
 )
 
 // trafficDirection defines, for filter terms, the direction of the traffic from an application
@@ -77,7 +76,7 @@ func getHTTPConnectionManager(routeName string, cfg configurator.Configurator, h
 		connManager.Tracing = tracing
 	}
 
-	if featureflags.IsWASMStatsEnabled() {
+	if cfg.GetFeatureFlags().EnableWASMStats {
 		statsFilter, err := getStatsWASMFilter()
 		if err != nil {
 			log.Error().Err(err).Msg("failed to get stats WASM filter")