diff --git a/.env.example b/.env.example index bdd7e5d318..abd5094a9f 100644 --- a/.env.example +++ b/.env.example @@ -106,10 +106,6 @@ export BOOKWAREHOUSE_NAMESPACE=bookwarehouse # Default: false # export DEPLOY_PROMETHEUS=true -# optional: ENABLE_PROMETHEUS_SCRAPING (true/false) -# Default: true -# export ENABLE_PROMETHEUS_SCRAPING=true - # optional: Maximum of iterations to test for expected return codes. 0 means unlimited. # export CI_MAX_ITERATIONS_THRESHOLD=0 diff --git a/charts/osm/README.md b/charts/osm/README.md index b8ae8078a3..05b476475c 100644 --- a/charts/osm/README.md +++ b/charts/osm/README.md @@ -75,7 +75,6 @@ The following table lists the configurable parameters of the osm chart and their | OpenServiceMesh.enableFluentbit | bool | `false` | Enable Fluent Bit sidecar deployment on OSM controller's pod | | OpenServiceMesh.enablePermissiveTrafficPolicy | bool | `false` | Enable permissive traffic policy mode | | OpenServiceMesh.enablePrivilegedInitContainer | bool | `false` | Run init container in privileged mode | -| OpenServiceMesh.enablePrometheusScraping | bool | `true` | Enable Prometheus metrics scraping on sidecar proxies | | OpenServiceMesh.enforceSingleMesh | bool | `false` | Enforce only deploying one mesh in the cluster | | OpenServiceMesh.envoyLogLevel | string | `"error"` | Log level for the Envoy proxy sidecar | | OpenServiceMesh.featureFlags.enableEgressPolicy | bool | `true` | Enable OSM's Egress policy API. If specified, fine grained control over Egress (external) traffic is enforced | diff --git a/charts/osm/crds/meshconfig.yaml b/charts/osm/crds/meshconfig.yaml index c04fd4b5da..de375edd6b 100644 --- a/charts/osm/crds/meshconfig.yaml +++ b/charts/osm/crds/meshconfig.yaml @@ -159,10 +159,6 @@ spec: description: Enables a debug endpoint on the osm-controller pod to list information regarding the mesh such as proxy connections, certificates, and SMI policies. type: boolean default: false - prometheusScraping: - description: Enables Prometheus metrics scraping on sidecar proxies. - type: boolean - default: true tracing: description: Configuration for distributed tracing type: object @@ -191,3 +187,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 index c34e939c13..72e41d1dd8 100644 --- 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 index c37bfb37ea..e59d6f9f0d 100644 --- a/charts/osm/templates/preset-mesh-config.yaml +++ b/charts/osm/templates/preset-mesh-config.yaml @@ -19,7 +19,6 @@ spec: outboundIPRangeExclusionList: {{.Values.OpenServiceMesh.outboundIPRangeExclusionList}} observability: enableDebugServer: {{.Values.OpenServiceMesh.enableDebugServer}} - prometheusScraping: {{.Values.OpenServiceMesh.enablePrometheusScraping}} tracing: enable: {{.Values.OpenServiceMesh.tracing.enable}} {{- if .Values.OpenServiceMesh.tracing.enable }} @@ -28,4 +27,8 @@ spec: endpoint: {{.Values.OpenServiceMesh.tracing.endpoint | quote}} {{- end }} certificate: - serviceCertValidityDuration: {{.Values.OpenServiceMesh.serviceCertValidityDuration}} \ No newline at end of file + serviceCertValidityDuration: {{.Values.OpenServiceMesh.serviceCertValidityDuration}} + featureFlags: + enableWASMStats: {{.Values.OpenServiceMesh.featureFlags.enableWASMStats}} + enableEgressPolicy: {{.Values.OpenServiceMesh.featureFlags.enableEgressPolicy}} + enableMulticlusterMode: {{.Values.OpenServiceMesh.featureFlags.enableMulticlusterMode}} diff --git a/charts/osm/values.schema.json b/charts/osm/values.schema.json index 9bf4635ee7..5fd9e55133 100644 --- a/charts/osm/values.schema.json +++ b/charts/osm/values.schema.json @@ -85,7 +85,6 @@ "enablePermissiveTrafficPolicy", "enableEgress", "deployPrometheus", - "enablePrometheusScraping", "deployGrafana", "enableFluentbit", "fluentBit", @@ -256,15 +255,6 @@ false ] }, - "enablePrometheusScraping": { - "$id": "#/properties/OpenServiceMesh/properties/enablePrometheusScraping", - "type": "boolean", - "title": "The enablePrometheusScraping schema", - "description": "Indicates whether Prometheus scraping should be enabled.", - "examples": [ - true - ] - }, "deployGrafana": { "$id": "#/properties/OpenServiceMesh/properties/deployGrafana", "type": "boolean", diff --git a/charts/osm/values.yaml b/charts/osm/values.yaml index 77b043ba2f..ad93bfd10b 100644 --- a/charts/osm/values.yaml +++ b/charts/osm/values.yaml @@ -104,9 +104,6 @@ OpenServiceMesh: # -- Deploy Prometheus with OSM installation deployPrometheus: false - # -- Enable Prometheus metrics scraping on sidecar proxies - enablePrometheusScraping: true - # -- Deploy Grafana with OSM installation deployGrafana: false diff --git a/cmd/cli/install.go b/cmd/cli/install.go index ea1257cd86..2245e09f46 100644 --- a/cmd/cli/install.go +++ b/cmd/cli/install.go @@ -233,13 +233,6 @@ func (i *installCmd) validateOptions() error { } if setOptions, ok := s["OpenServiceMesh"].(map[string]interface{}); ok { - // if deployPrometheus is true, make sure enablePrometheusScraping is not disabled - if setOptions["deployPrometheus"] == true { - if setOptions["enablePrometheusScraping"] == false { - _, _ = fmt.Fprintf(i.out, "Prometheus scraping is disabled. To enable it, set prometheus_scraping in %s/%s to true.\n", settings.Namespace(), constants.OSMMeshConfig) - } - } - // if certificateManager is vault, ensure all relevant information (vault-host, vault-token) is available if setOptions["certificateManager"] == "vault" { var missingFields []string diff --git a/cmd/cli/install_test.go b/cmd/cli/install_test.go index 6f61a34dd6..79aa231f1c 100644 --- a/cmd/cli/install_test.go +++ b/cmd/cli/install_test.go @@ -534,7 +534,6 @@ var _ = Describe("deployPrometheus is true", func() { installCmd := getDefaultInstallCmd(out) installCmd.setOptions = []string{ "OpenServiceMesh.deployPrometheus=true", - "OpenServiceMesh.enablePrometheusScraping=false", } err = installCmd.run(config) @@ -542,7 +541,6 @@ var _ = Describe("deployPrometheus is true", func() { It("should not error", func() { Expect(err).NotTo(HaveOccurred()) - Expect(out.String()).To(Equal("Prometheus scraping is disabled. To enable it, set prometheus_scraping in osm-system/osm-mesh-config to true.\nOSM installed successfully in namespace [osm-system] with mesh name [osm]\n")) }) }) diff --git a/cmd/init-osm-controller/init-osm-controller_test.go b/cmd/init-osm-controller/init-osm-controller_test.go index ffd27df8ec..a767515d89 100644 --- a/cmd/init-osm-controller/init-osm-controller_test.go +++ b/cmd/init-osm-controller/init-osm-controller_test.go @@ -35,8 +35,7 @@ func TestCreateDefaultMeshConfig(t *testing.T) { EnablePermissiveTrafficPolicyMode: true, }, Observability: v1alpha1.ObservabilitySpec{ - EnableDebugServer: false, - PrometheusScraping: true, + EnableDebugServer: false, Tracing: v1alpha1.TracingSpec{ Enable: false, }, @@ -55,7 +54,6 @@ func TestCreateDefaultMeshConfig(t *testing.T) { assert.Equal(meshConfig.Spec.Traffic.EnablePermissiveTrafficPolicyMode, true) assert.Equal(meshConfig.Spec.Traffic.EnableEgress, true) assert.Equal(meshConfig.Spec.Traffic.UseHTTPSIngress, false) - assert.Equal(meshConfig.Spec.Observability.PrometheusScraping, true) assert.Equal(meshConfig.Spec.Observability.EnableDebugServer, false) assert.Equal(meshConfig.Spec.Certificate.ServiceCertValidityDuration, "24h") } diff --git a/cmd/osm-controller/osm-controller.go b/cmd/osm-controller/osm-controller.go index 42d57361a3..6d65b1b592 100644 --- 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/demo/run-osm-demo.sh b/demo/run-osm-demo.sh index e810d04fb0..2e43c78581 100755 --- a/demo/run-osm-demo.sh +++ b/demo/run-osm-demo.sh @@ -33,7 +33,6 @@ DEPLOY_GRAFANA="${DEPLOY_GRAFANA:-false}" DEPLOY_JAEGER="${DEPLOY_JAEGER:-false}" ENABLE_FLUENTBIT="${ENABLE_FLUENTBIT:-false}" DEPLOY_PROMETHEUS="${DEPLOY_PROMETHEUS:-false}" -ENABLE_PROMETHEUS_SCRAPING="${ENABLE_PROMETHEUS_SCRAPING:-true}" DEPLOY_WITH_SAME_SA="${DEPLOY_WITH_SAME_SA:-false}" ENVOY_LOG_LEVEL="${ENVOY_LOG_LEVEL:-debug}" DEPLOY_ON_OPENSHIFT="${DEPLOY_ON_OPENSHIFT:-false}" @@ -107,7 +106,6 @@ if [ "$CERT_MANAGER" = "vault" ]; then --set=OpenServiceMesh.deployJaeger="$DEPLOY_JAEGER" \ --set=OpenServiceMesh.enableFluentbit="$ENABLE_FLUENTBIT" \ --set=OpenServiceMesh.deployPrometheus="$DEPLOY_PROMETHEUS" \ - --set=OpenServiceMesh.enablePrometheusScraping="$ENABLE_PROMETHEUS_SCRAPING" \ --set=OpenServiceMesh.envoyLogLevel="$ENVOY_LOG_LEVEL" \ --set=OpenServiceMesh.controllerLogLevel="trace" \ --timeout=90s \ @@ -128,7 +126,6 @@ else --set=OpenServiceMesh.deployJaeger="$DEPLOY_JAEGER" \ --set=OpenServiceMesh.enableFluentbit="$ENABLE_FLUENTBIT" \ --set=OpenServiceMesh.deployPrometheus="$DEPLOY_PROMETHEUS" \ - --set=OpenServiceMesh.enablePrometheusScraping="$ENABLE_PROMETHEUS_SCRAPING" \ --set=OpenServiceMesh.envoyLogLevel="$ENVOY_LOG_LEVEL" \ --set=OpenServiceMesh.controllerLogLevel="trace" \ --timeout=90s \ diff --git a/docs/example/manifests/meshconfig/mesh-config.yaml b/docs/example/manifests/meshconfig/mesh-config.yaml index 81dfb5498c..9531af3437 100644 --- a/docs/example/manifests/meshconfig/mesh-config.yaml +++ b/docs/example/manifests/meshconfig/mesh-config.yaml @@ -16,7 +16,6 @@ spec: enablePermissiveTrafficPolicyMode: true observability: enableDebugServer: true - prometheusScraping: true outboundPortExclusionList: [] inboundPortExclusionList: [] outboundIPRangeExclusionList: [] diff --git a/pkg/apis/config/v1alpha1/mesh_config.go b/pkg/apis/config/v1alpha1/mesh_config.go index 72ff0f036b..70092b216c 100644 --- 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. @@ -91,9 +94,6 @@ type ObservabilitySpec struct { // EnableDebugServer defines if the debug endpoint on the OSM controller pod is enabled. EnableDebugServer bool `json:"enableDebugServer,omitempty"` - // PrometheusScraping defines a boolean indicating if sidecars should be configured for Prometheus metrics scraping. - PrometheusScraping bool `json:"prometheusScraping,omitempty"` - // Tracing defines OSM's tracing configuration. Tracing TracingSpec `json:"tracing,omitempty"` } @@ -150,3 +150,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/catalog.go b/pkg/catalog/catalog.go index 55336386ef..fc71c43130 100644 --- a/pkg/catalog/catalog.go +++ b/pkg/catalog/catalog.go @@ -36,3 +36,8 @@ func NewMeshCatalog(kubeController k8s.Controller, kubeClient kubernetes.Interfa return &mc } + +// GetKubeController returns the kube controller instance handling the current cluster +func (mc *MeshCatalog) GetKubeController() k8s.Controller { + return mc.kubeController +} diff --git a/pkg/catalog/egress.go b/pkg/catalog/egress.go index e648fd75b4..20531026c6 100644 --- a/pkg/catalog/egress.go +++ b/pkg/catalog/egress.go @@ -11,7 +11,6 @@ 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" @@ -19,7 +18,7 @@ import ( // 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 index a12f057e98..f02defec7a 100644 --- 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/fake.go b/pkg/catalog/fake.go index 98f5c70999..17e461ed33 100644 --- a/pkg/catalog/fake.go +++ b/pkg/catalog/fake.go @@ -109,6 +109,7 @@ func NewFakeMeshCatalog(kubeClient kubernetes.Interface, meshConfigClient versio mockKubeController.EXPECT().ListServiceIdentitiesForService(tests.BookstoreV1Service).Return([]identity.K8sServiceAccount{tests.BookstoreServiceAccount}, nil).AnyTimes() mockKubeController.EXPECT().ListServiceIdentitiesForService(tests.BookstoreV2Service).Return([]identity.K8sServiceAccount{tests.BookstoreV2ServiceAccount}, nil).AnyTimes() mockKubeController.EXPECT().ListServiceIdentitiesForService(tests.BookbuyerService).Return([]identity.K8sServiceAccount{tests.BookbuyerServiceAccount}, nil).AnyTimes() + mockKubeController.EXPECT().IsMetricsEnabled(gomock.Any()).Return(true).AnyTimes() mockPolicyController.EXPECT().ListEgressPoliciesForSourceIdentity(gomock.Any()).Return(nil).AnyTimes() diff --git a/pkg/catalog/mock_catalog_generated.go b/pkg/catalog/mock_catalog_generated.go index 3f5b49f246..e61bf7713f 100644 --- a/pkg/catalog/mock_catalog_generated.go +++ b/pkg/catalog/mock_catalog_generated.go @@ -10,6 +10,7 @@ import ( gomock "github.com/golang/mock/gomock" endpoint "github.com/openservicemesh/osm/pkg/endpoint" identity "github.com/openservicemesh/osm/pkg/identity" + kubernetes "github.com/openservicemesh/osm/pkg/kubernetes" service "github.com/openservicemesh/osm/pkg/service" trafficpolicy "github.com/openservicemesh/osm/pkg/trafficpolicy" ) @@ -67,6 +68,20 @@ func (mr *MockMeshCatalogerMockRecorder) GetIngressPoliciesForService(arg0 inter return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetIngressPoliciesForService", reflect.TypeOf((*MockMeshCataloger)(nil).GetIngressPoliciesForService), arg0) } +// GetKubeController mocks base method +func (m *MockMeshCataloger) GetKubeController() kubernetes.Controller { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetKubeController") + ret0, _ := ret[0].(kubernetes.Controller) + return ret0 +} + +// GetKubeController indicates an expected call of GetKubeController +func (mr *MockMeshCatalogerMockRecorder) GetKubeController() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetKubeController", reflect.TypeOf((*MockMeshCataloger)(nil).GetKubeController)) +} + // GetPortToProtocolMappingForService mocks base method func (m *MockMeshCataloger) GetPortToProtocolMappingForService(arg0 service.MeshService) (map[uint32]string, error) { m.ctrl.T.Helper() diff --git a/pkg/catalog/types.go b/pkg/catalog/types.go index a70d74935f..5936d0c7b7 100644 --- a/pkg/catalog/types.go +++ b/pkg/catalog/types.go @@ -97,8 +97,11 @@ 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 + GetKubeController() k8s.Controller } type trafficDirection string diff --git a/pkg/configurator/methods.go b/pkg/configurator/methods.go index 38b1a6b62b..a03fffa59a 100644 --- a/pkg/configurator/methods.go +++ b/pkg/configurator/methods.go @@ -60,11 +60,6 @@ func (c *Client) IsDebugServerEnabled() bool { return c.getMeshConfig().Spec.Observability.EnableDebugServer } -// IsPrometheusScrapingEnabled determines whether Prometheus is enabled for scraping metrics -func (c *Client) IsPrometheusScrapingEnabled() bool { - return c.getMeshConfig().Spec.Observability.PrometheusScraping -} - // IsTracingEnabled returns whether tracing is enabled func (c *Client) IsTracingEnabled() bool { return c.getMeshConfig().Spec.Observability.Tracing.Enable @@ -203,3 +198,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 index b3c7c9c1ff..553f4d39dd 100644 --- a/pkg/configurator/methods_test.go +++ b/pkg/configurator/methods_test.go @@ -62,8 +62,7 @@ func TestCreateUpdateConfig(t *testing.T) { UseHTTPSIngress: true, }, Observability: v1alpha1.ObservabilitySpec{ - EnableDebugServer: true, - PrometheusScraping: true, + EnableDebugServer: true, Tracing: v1alpha1.TracingSpec{ Enable: true, }, @@ -88,8 +87,7 @@ func TestCreateUpdateConfig(t *testing.T) { UseHTTPSIngress: true, }, Observability: v1alpha1.ObservabilitySpec{ - EnableDebugServer: true, - PrometheusScraping: true, + EnableDebugServer: true, Tracing: v1alpha1.TracingSpec{ Enable: true, }, @@ -163,25 +161,6 @@ func TestCreateUpdateConfig(t *testing.T) { assert.False(cfg.IsDebugServerEnabled()) }, }, - { - name: "IsPrometheusScrapingEnabled", - initialMeshConfigData: &v1alpha1.MeshConfigSpec{ - Observability: v1alpha1.ObservabilitySpec{ - PrometheusScraping: true, - }, - }, - checkCreate: func(assert *tassert.Assertions, cfg Configurator) { - assert.True(cfg.IsPrometheusScrapingEnabled()) - }, - updatedMeshConfigData: &v1alpha1.MeshConfigSpec{ - Observability: v1alpha1.ObservabilitySpec{ - PrometheusScraping: false, - }, - }, - checkUpdate: func(assert *tassert.Assertions, cfg Configurator) { - assert.False(cfg.IsPrometheusScrapingEnabled()) - }, - }, { name: "IsTracingEnabled", initialMeshConfigData: &v1alpha1.MeshConfigSpec{ @@ -440,6 +419,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 index ecd5a0dc1b..b7e514e206 100644 --- a/pkg/configurator/mock_client_generated.go +++ b/pkg/configurator/mock_client_generated.go @@ -9,6 +9,7 @@ import ( time "time" gomock "github.com/golang/mock/gomock" + v1alpha1 "github.com/openservicemesh/osm/pkg/apis/config/v1alpha1" auth "github.com/openservicemesh/osm/pkg/auth" v1 "k8s.io/api/core/v1" ) @@ -78,6 +79,20 @@ func (mr *MockConfiguratorMockRecorder) GetEnvoyLogLevel() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetEnvoyLogLevel", reflect.TypeOf((*MockConfigurator)(nil).GetEnvoyLogLevel)) } +// GetFeatureFlags mocks base method +func (m *MockConfigurator) GetFeatureFlags() v1alpha1.FeatureFlags { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetFeatureFlags") + ret0, _ := ret[0].(v1alpha1.FeatureFlags) + return ret0 +} + +// GetFeatureFlags indicates an expected call of GetFeatureFlags +func (mr *MockConfiguratorMockRecorder) GetFeatureFlags() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetFeatureFlags", reflect.TypeOf((*MockConfigurator)(nil).GetFeatureFlags)) +} + // GetInboundExternalAuthConfig mocks base method func (m *MockConfigurator) GetInboundExternalAuthConfig() auth.ExtAuthConfig { m.ctrl.T.Helper() @@ -317,20 +332,6 @@ func (mr *MockConfiguratorMockRecorder) IsPrivilegedInitContainer() *gomock.Call return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsPrivilegedInitContainer", reflect.TypeOf((*MockConfigurator)(nil).IsPrivilegedInitContainer)) } -// IsPrometheusScrapingEnabled mocks base method -func (m *MockConfigurator) IsPrometheusScrapingEnabled() bool { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "IsPrometheusScrapingEnabled") - ret0, _ := ret[0].(bool) - return ret0 -} - -// IsPrometheusScrapingEnabled indicates an expected call of IsPrometheusScrapingEnabled -func (mr *MockConfiguratorMockRecorder) IsPrometheusScrapingEnabled() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsPrometheusScrapingEnabled", reflect.TypeOf((*MockConfigurator)(nil).IsPrometheusScrapingEnabled)) -} - // IsTracingEnabled mocks base method func (m *MockConfigurator) IsTracingEnabled() bool { m.ctrl.T.Helper() diff --git a/pkg/configurator/types.go b/pkg/configurator/types.go index ed5dc68d5d..6c7c5f73d3 100644 --- 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" ) @@ -42,9 +43,6 @@ type Configurator interface { // IsDebugServerEnabled determines whether osm debug HTTP server is enabled IsDebugServerEnabled() bool - // IsPrometheusScrapingEnabled determines whether Prometheus is enabled for scraping metrics - IsPrometheusScrapingEnabled() bool - // IsTracingEnabled returns whether tracing is enabled IsTracingEnabled() bool @@ -96,4 +94,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 index 649c4fed57..e69cb570b4 100644 --- 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 index 8010d82242..34b1736180 100644 --- 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" @@ -124,11 +125,14 @@ var _ = Describe("Test ADS response functions", func() { server, actualResponses := tests.NewFakeXDSServer(cert, nil, nil) mockConfigurator.EXPECT().IsEgressEnabled().Return(false).AnyTimes() - mockConfigurator.EXPECT().IsPrometheusScrapingEnabled().Return(false).AnyTimes() mockConfigurator.EXPECT().IsTracingEnabled().Return(false).AnyTimes() 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) @@ -204,7 +208,6 @@ var _ = Describe("Test ADS response functions", func() { server, actualResponses := tests.NewFakeXDSServer(cert, nil, nil) mockConfigurator.EXPECT().IsEgressEnabled().Return(false).AnyTimes() - mockConfigurator.EXPECT().IsPrometheusScrapingEnabled().Return(false).AnyTimes() mockConfigurator.EXPECT().IsTracingEnabled().Return(false).AnyTimes() mockConfigurator.EXPECT().IsPermissiveTrafficPolicyMode().Return(false).AnyTimes() mockConfigurator.EXPECT().GetServiceCertValidityPeriod().Return(certDuration).AnyTimes() diff --git a/pkg/envoy/cds/response.go b/pkg/envoy/cds/response.go index e3f1d20e23..afaf210f84 100644 --- a/pkg/envoy/cds/response.go +++ b/pkg/envoy/cds/response.go @@ -75,7 +75,9 @@ func NewResponse(meshCatalog catalog.MeshCataloger, proxy *envoy.Proxy, _ *xds_d } // Add an inbound prometheus cluster (from Prometheus to localhost) - if cfg.IsPrometheusScrapingEnabled() { + if pod, err := envoy.GetPodFromCertificate(proxy.GetCertificateCommonName(), meshCatalog.GetKubeController()); err != nil { + log.Warn().Msgf("Could not find pod for connecting proxy %s. No metadata was recorded.", proxy.GetCertificateSerialNumber()) + } else if meshCatalog.GetKubeController().IsMetricsEnabled(pod) { clusters = append(clusters, getPrometheusCluster()) } diff --git a/pkg/envoy/cds/response_test.go b/pkg/envoy/cds/response_test.go index 37d48d27b9..a26de1d5de 100644 --- a/pkg/envoy/cds/response_test.go +++ b/pkg/envoy/cds/response_test.go @@ -1,6 +1,7 @@ package cds import ( + "context" "fmt" "testing" "time" @@ -19,6 +20,9 @@ import ( tassert "github.com/stretchr/testify/assert" trequire "github.com/stretchr/testify/require" "google.golang.org/protobuf/testing/protocmp" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + testclient "k8s.io/client-go/kubernetes/fake" "github.com/openservicemesh/osm/pkg/catalog" "github.com/openservicemesh/osm/pkg/certificate" @@ -27,6 +31,7 @@ import ( "github.com/openservicemesh/osm/pkg/envoy" "github.com/openservicemesh/osm/pkg/envoy/registry" "github.com/openservicemesh/osm/pkg/envoy/secrets" + k8s "github.com/openservicemesh/osm/pkg/kubernetes" "github.com/openservicemesh/osm/pkg/service" "github.com/openservicemesh/osm/pkg/tests" ) @@ -36,8 +41,10 @@ func TestNewResponse(t *testing.T) { require := trequire.New(t) mockCtrl := gomock.NewController(t) + kubeClient := testclient.NewSimpleClientset() mockConfigurator := configurator.NewMockConfigurator(mockCtrl) mockCatalog := catalog.NewMockMeshCataloger(mockCtrl) + mockKubeController := k8s.NewMockController(mockCtrl) proxyUUID := uuid.New() // The format of the CN matters @@ -53,10 +60,25 @@ func TestNewResponse(t *testing.T) { mockCatalog.EXPECT().GetEgressTrafficPolicy(tests.BookbuyerServiceIdentity).Return(nil, nil).AnyTimes() mockConfigurator.EXPECT().IsPermissiveTrafficPolicyMode().Return(false).AnyTimes() mockConfigurator.EXPECT().IsEgressEnabled().Return(true).AnyTimes() - mockConfigurator.EXPECT().IsPrometheusScrapingEnabled().Return(true).AnyTimes() mockConfigurator.EXPECT().IsTracingEnabled().Return(true).AnyTimes() mockConfigurator.EXPECT().GetTracingHost().Return(constants.DefaultTracingHost).AnyTimes() mockConfigurator.EXPECT().GetTracingPort().Return(constants.DefaultTracingPort).AnyTimes() + mockCatalog.EXPECT().GetKubeController().Return(mockKubeController).AnyTimes() + + podlabels := map[string]string{ + tests.SelectorKey: tests.BookbuyerServiceName, + constants.EnvoyUniqueIDLabelName: proxyUUID.String(), + } + + newPod1 := tests.NewPodFixture(tests.Namespace, fmt.Sprintf("pod-1-%s", proxyUUID), tests.BookbuyerServiceAccountName, podlabels) + newPod1.Annotations = map[string]string{ + constants.PrometheusScrapeAnnotation: "true", + } + _, err := kubeClient.CoreV1().Pods(tests.Namespace).Create(context.TODO(), &newPod1, metav1.CreateOptions{}) + assert.Nil(err) + + mockKubeController.EXPECT().ListPods().Return([]*v1.Pod{&newPod1}) + mockKubeController.EXPECT().IsMetricsEnabled(&newPod1).Return(true) resp, err := NewResponse(mockCatalog, proxy, nil, mockConfigurator, nil, proxyRegistry) assert.Nil(err) diff --git a/pkg/envoy/lds/connection_manager.go b/pkg/envoy/lds/connection_manager.go index 7fee1e36a9..738119ad20 100644 --- 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") diff --git a/pkg/envoy/lds/egress_test.go b/pkg/envoy/lds/egress_test.go index 94145d6981..0d9e543ddf 100644 --- a/pkg/envoy/lds/egress_test.go +++ b/pkg/envoy/lds/egress_test.go @@ -10,24 +10,19 @@ import ( tassert "github.com/stretchr/testify/assert" "google.golang.org/protobuf/types/known/wrapperspb" + "github.com/openservicemesh/osm/pkg/apis/config/v1alpha1" "github.com/openservicemesh/osm/pkg/configurator" - "github.com/openservicemesh/osm/pkg/featureflags" "github.com/openservicemesh/osm/pkg/trafficpolicy" ) -func init() { - // Initialize the Egress policy feature - featureflags.Initialize(featureflags.OptionalFeatures{ - EgressPolicy: true, - }) -} - func TestGetEgressHTTPFilterChain(t *testing.T) { assert := tassert.New(t) mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() + mockConfigurator := configurator.NewMockConfigurator(mockCtrl) + testCases := []struct { name string destinationPort int @@ -56,12 +51,13 @@ func TestGetEgressHTTPFilterChain(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - mockConfigurator := configurator.NewMockConfigurator(mockCtrl) lb := &listenerBuilder{ cfg: mockConfigurator, } mockConfigurator.EXPECT().IsTracingEnabled().Return(false).AnyTimes() - + mockConfigurator.EXPECT().GetFeatureFlags().Return(v1alpha1.FeatureFlags{ + EnableEgressPolicy: true, + EnableWASMStats: false}).AnyTimes() actual, err := lb.getEgressHTTPFilterChain(tc.destinationPort) assert.Equal(tc.expectError, err != nil) @@ -78,6 +74,8 @@ func TestGetEgressTCPFilterChain(t *testing.T) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() + mockConfigurator := configurator.NewMockConfigurator(mockCtrl) + testCases := []struct { name string trafficMatch trafficpolicy.TrafficMatch @@ -153,6 +151,11 @@ func TestGetEgressTCPFilterChain(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { + mockConfigurator.EXPECT().GetFeatureFlags().Return(v1alpha1.FeatureFlags{ + EnableEgressPolicy: true, + EnableWASMStats: false, + }).AnyTimes() + lb := &listenerBuilder{} actual, err := lb.getEgressTCPFilterChain(tc.trafficMatch) @@ -171,6 +174,8 @@ func TestGetEgressFilterChainsForMatches(t *testing.T) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() + mockConfigurator := configurator.NewMockConfigurator(mockCtrl) + testCases := []struct { name string trafficMatches []*trafficpolicy.TrafficMatch @@ -217,11 +222,14 @@ func TestGetEgressFilterChainsForMatches(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - mockConfigurator := configurator.NewMockConfigurator(mockCtrl) lb := &listenerBuilder{ cfg: mockConfigurator, } mockConfigurator.EXPECT().IsTracingEnabled().Return(false).AnyTimes() + mockConfigurator.EXPECT().GetFeatureFlags().Return(v1alpha1.FeatureFlags{ + EnableEgressPolicy: true, + EnableWASMStats: false, + }).AnyTimes() actual := lb.getEgressFilterChainsForMatches(tc.trafficMatches) diff --git a/pkg/envoy/lds/ingress_test.go b/pkg/envoy/lds/ingress_test.go index 2d19cabee9..a98a08ca59 100644 --- a/pkg/envoy/lds/ingress_test.go +++ b/pkg/envoy/lds/ingress_test.go @@ -11,6 +11,7 @@ import ( "github.com/envoyproxy/go-control-plane/pkg/wellknown" "google.golang.org/protobuf/types/known/wrapperspb" + "github.com/openservicemesh/osm/pkg/apis/config/v1alpha1" "github.com/openservicemesh/osm/pkg/auth" "github.com/openservicemesh/osm/pkg/catalog" "github.com/openservicemesh/osm/pkg/configurator" @@ -108,6 +109,8 @@ func TestGetIngressFilterChains(t *testing.T) { mockConfigurator.EXPECT().GetInboundExternalAuthConfig().Return(auth.ExtAuthConfig{ Enable: false, }).AnyTimes() + // Mock configurator call to determine if WASMStats are enabled + mockConfigurator.EXPECT().GetFeatureFlags().Return(v1alpha1.FeatureFlags{EnableWASMStats: false}).AnyTimes() filterChains := lb.getIngressFilterChains(proxyService) diff --git a/pkg/envoy/lds/inmesh_test.go b/pkg/envoy/lds/inmesh_test.go index 29629edd5a..84d14d8554 100644 --- a/pkg/envoy/lds/inmesh_test.go +++ b/pkg/envoy/lds/inmesh_test.go @@ -14,6 +14,7 @@ import ( tassert "github.com/stretchr/testify/assert" "google.golang.org/protobuf/types/known/wrapperspb" + "github.com/openservicemesh/osm/pkg/apis/config/v1alpha1" "github.com/openservicemesh/osm/pkg/auth" "github.com/openservicemesh/osm/pkg/catalog" "github.com/openservicemesh/osm/pkg/configurator" @@ -38,6 +39,9 @@ func TestGetOutboundHTTPFilterChainForService(t *testing.T) { mockConfigurator.EXPECT().GetInboundExternalAuthConfig().Return(auth.ExtAuthConfig{ Enable: false, }).AnyTimes() + mockConfigurator.EXPECT().GetFeatureFlags().Return(v1alpha1.FeatureFlags{ + EnableWASMStats: false, + }).AnyTimes() lb := &listenerBuilder{ meshCatalog: mockCatalog, @@ -203,6 +207,9 @@ func TestGetInboundMeshHTTPFilterChain(t *testing.T) { mockConfigurator.EXPECT().GetInboundExternalAuthConfig().Return(auth.ExtAuthConfig{ Enable: false, }).AnyTimes() + mockConfigurator.EXPECT().GetFeatureFlags().Return(v1alpha1.FeatureFlags{ + EnableWASMStats: false, + }).AnyTimes() lb := &listenerBuilder{ meshCatalog: mockCatalog, diff --git a/pkg/envoy/lds/listener.go b/pkg/envoy/lds/listener.go index 0a4eb7674b..5bd61a080a 100644 --- a/pkg/envoy/lds/listener.go +++ b/pkg/envoy/lds/listener.go @@ -14,7 +14,6 @@ import ( "github.com/openservicemesh/osm/pkg/constants" "github.com/openservicemesh/osm/pkg/envoy" - "github.com/openservicemesh/osm/pkg/featureflags" "github.com/openservicemesh/osm/pkg/trafficpolicy" ) @@ -57,7 +56,7 @@ func (lb *listenerBuilder) newOutboundListener() (*xds_listener.Listener, error) listener.DefaultFilterChain = egressFilterChain } - if featureflags.IsEgressPolicyEnabled() { + if featureflags := lb.cfg.GetFeatureFlags(); featureflags.EnableEgressPolicy { var filterDisableMatchPredicate *xds_listener.ListenerFilterChainMatchPredicate // Create filter chains for egress based on policies if egressTrafficPolicy, err := lb.meshCatalog.GetEgressTrafficPolicy(lb.serviceIdentity); err != nil { diff --git a/pkg/envoy/lds/listener_test.go b/pkg/envoy/lds/listener_test.go index a0f24c5bc3..863fe8e3de 100644 --- a/pkg/envoy/lds/listener_test.go +++ b/pkg/envoy/lds/listener_test.go @@ -14,12 +14,12 @@ import ( . "github.com/onsi/gomega" tassert "github.com/stretchr/testify/assert" + "github.com/openservicemesh/osm/pkg/apis/config/v1alpha1" "github.com/openservicemesh/osm/pkg/auth" "github.com/openservicemesh/osm/pkg/configurator" "github.com/openservicemesh/osm/pkg/constants" "github.com/openservicemesh/osm/pkg/envoy" "github.com/openservicemesh/osm/pkg/envoy/rds/route" - "github.com/openservicemesh/osm/pkg/featureflags" "github.com/openservicemesh/osm/pkg/trafficpolicy" ) @@ -42,6 +42,9 @@ func TestGetFilterForService(t *testing.T) { mockConfigurator.EXPECT().GetInboundExternalAuthConfig().Return(auth.ExtAuthConfig{ Enable: false, }).AnyTimes() + mockConfigurator.EXPECT().GetFeatureFlags().Return(v1alpha1.FeatureFlags{ + EnableWASMStats: false, + }).AnyTimes() // Check we get HTTP connection manager filter without Permissive mode filter, err := lb.getOutboundHTTPFilter(route.OutboundRouteConfigName) @@ -108,10 +111,16 @@ var _ = Describe("Test getHTTPConnectionManager", func() { It("Should have the correct StatPrefix", func() { mockConfigurator.EXPECT().IsTracingEnabled().Return(false).Times(1) + mockConfigurator.EXPECT().GetFeatureFlags().Return(v1alpha1.FeatureFlags{ + EnableWASMStats: false, + }).Times(1) connManager := getHTTPConnectionManager("foo", mockConfigurator, nil, outbound) Expect(connManager.StatPrefix).To(Equal("mesh-http-conn-manager.foo")) mockConfigurator.EXPECT().IsTracingEnabled().Return(false).Times(1) + mockConfigurator.EXPECT().GetFeatureFlags().Return(v1alpha1.FeatureFlags{ + EnableWASMStats: false, + }).Times(1) connManager = getHTTPConnectionManager("bar", mockConfigurator, nil, outbound) Expect(connManager.StatPrefix).To(Equal("mesh-http-conn-manager.bar")) }) @@ -121,6 +130,9 @@ var _ = Describe("Test getHTTPConnectionManager", func() { mockConfigurator.EXPECT().GetTracingPort().Return(constants.DefaultTracingPort).Times(1) mockConfigurator.EXPECT().GetTracingEndpoint().Return(constants.DefaultTracingEndpoint).Times(1) mockConfigurator.EXPECT().IsTracingEnabled().Return(true).Times(1) + mockConfigurator.EXPECT().GetFeatureFlags().Return(v1alpha1.FeatureFlags{ + EnableWASMStats: false, + }).Times(1) connManager := getHTTPConnectionManager(route.InboundRouteConfigName, mockConfigurator, nil, outbound) @@ -130,6 +142,9 @@ var _ = Describe("Test getHTTPConnectionManager", func() { It("Returns proper Zipkin config given when tracing is disabled", func() { mockConfigurator.EXPECT().IsTracingEnabled().Return(false).Times(1) + mockConfigurator.EXPECT().GetFeatureFlags().Return(v1alpha1.FeatureFlags{ + EnableWASMStats: false, + }).Times(1) connManager := getHTTPConnectionManager(route.InboundRouteConfigName, mockConfigurator, nil, outbound) var nilHcmTrace *xds_hcm.HttpConnectionManager_Tracing = nil @@ -139,9 +154,9 @@ var _ = Describe("Test getHTTPConnectionManager", func() { It("Returns no stats config when WASM is disabled", func() { mockConfigurator.EXPECT().IsTracingEnabled().AnyTimes() - - oldWASMflag := featureflags.Features.WASMStats - featureflags.Features.WASMStats = false + mockConfigurator.EXPECT().GetFeatureFlags().Return(v1alpha1.FeatureFlags{ + EnableWASMStats: false, + }).Times(1) oldStatsWASMBytes := statsWASMBytes statsWASMBytes = testWASM @@ -155,14 +170,13 @@ var _ = Describe("Test getHTTPConnectionManager", func() { // reset global state statsWASMBytes = oldStatsWASMBytes - featureflags.Features.WASMStats = oldWASMflag }) It("Returns no stats config when WASM is disabled and no WASM is defined", func() { mockConfigurator.EXPECT().IsTracingEnabled().AnyTimes() - - oldWASMflag := featureflags.Features.WASMStats - featureflags.Features.WASMStats = true + mockConfigurator.EXPECT().GetFeatureFlags().Return(v1alpha1.FeatureFlags{ + EnableWASMStats: true, + }).Times(1) oldStatsWASMBytes := statsWASMBytes statsWASMBytes = "" @@ -176,14 +190,13 @@ var _ = Describe("Test getHTTPConnectionManager", func() { // reset global state statsWASMBytes = oldStatsWASMBytes - featureflags.Features.WASMStats = oldWASMflag }) It("Returns no Lua headers filter config when there are no headers to add", func() { mockConfigurator.EXPECT().IsTracingEnabled().AnyTimes() - - oldWASMflag := featureflags.Features.WASMStats - featureflags.Features.WASMStats = true + mockConfigurator.EXPECT().GetFeatureFlags().Return(v1alpha1.FeatureFlags{ + EnableWASMStats: true, + }).Times(1) oldStatsWASMBytes := statsWASMBytes statsWASMBytes = testWASM @@ -198,14 +211,13 @@ var _ = Describe("Test getHTTPConnectionManager", func() { // reset global state statsWASMBytes = oldStatsWASMBytes - featureflags.Features.WASMStats = oldWASMflag }) It("Returns proper stats config when WASM is enabled", func() { mockConfigurator.EXPECT().IsTracingEnabled().AnyTimes() - - oldWASMflag := featureflags.Features.WASMStats - featureflags.Features.WASMStats = true + mockConfigurator.EXPECT().GetFeatureFlags().Return(v1alpha1.FeatureFlags{ + EnableWASMStats: true, + }).Times(1) oldStatsWASMBytes := statsWASMBytes statsWASMBytes = testWASM @@ -222,7 +234,6 @@ var _ = Describe("Test getHTTPConnectionManager", func() { // reset global state statsWASMBytes = oldStatsWASMBytes - featureflags.Features.WASMStats = oldWASMflag }) It("Returns inbound external authorization enabled connection manager when enabled by config", func() { @@ -235,6 +246,9 @@ var _ = Describe("Test getHTTPConnectionManager", func() { AuthzTimeout: 3 * time.Second, FailureModeAllow: false, }).Times(1) + mockConfigurator.EXPECT().GetFeatureFlags().Return(v1alpha1.FeatureFlags{ + EnableWASMStats: false, + }).Times(1) connManager := getHTTPConnectionManager(route.InboundRouteConfigName, mockConfigurator, nil, inbound) diff --git a/pkg/envoy/lds/response.go b/pkg/envoy/lds/response.go index 2333d5cad0..cac1e08ee5 100644 --- a/pkg/envoy/lds/response.go +++ b/pkg/envoy/lds/response.go @@ -9,7 +9,6 @@ import ( "github.com/openservicemesh/osm/pkg/configurator" "github.com/openservicemesh/osm/pkg/envoy" "github.com/openservicemesh/osm/pkg/envoy/registry" - "github.com/openservicemesh/osm/pkg/featureflags" "github.com/openservicemesh/osm/pkg/identity" ) @@ -34,7 +33,7 @@ func NewResponse(meshCatalog catalog.MeshCataloger, proxy *envoy.Proxy, _ *xds_d var ldsResources []types.Resource var statsHeaders map[string]string - if featureflags.IsWASMStatsEnabled() { + if featureflags := cfg.GetFeatureFlags(); featureflags.EnableWASMStats { statsHeaders = proxy.StatsHeaders() } @@ -86,7 +85,9 @@ func NewResponse(meshCatalog catalog.MeshCataloger, proxy *envoy.Proxy, _ *xds_d ldsResources = append(ldsResources, inboundListener) } - if cfg.IsPrometheusScrapingEnabled() { + if pod, err := envoy.GetPodFromCertificate(proxy.GetCertificateCommonName(), meshCatalog.GetKubeController()); err != nil { + log.Warn().Msgf("Could not find pod for connecting proxy %s. No metadata was recorded.", proxy.GetCertificateSerialNumber()) + } else if meshCatalog.GetKubeController().IsMetricsEnabled(pod) { // Build Prometheus listener config prometheusConnManager := getPrometheusConnectionManager() if prometheusListener, err := buildPrometheusListener(prometheusConnManager); err != nil { diff --git a/pkg/envoy/lds/response_test.go b/pkg/envoy/lds/response_test.go index 36148a83b0..98da39b430 100644 --- a/pkg/envoy/lds/response_test.go +++ b/pkg/envoy/lds/response_test.go @@ -1,6 +1,7 @@ package lds import ( + "context" "fmt" "testing" @@ -9,9 +10,11 @@ import ( "github.com/envoyproxy/go-control-plane/pkg/wellknown" "github.com/golang/mock/gomock" tassert "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" testclient "k8s.io/client-go/kubernetes/fake" + "github.com/openservicemesh/osm/pkg/apis/config/v1alpha1" "github.com/openservicemesh/osm/pkg/auth" "github.com/openservicemesh/osm/pkg/envoy/registry" configFake "github.com/openservicemesh/osm/pkg/gen/client/config/clientset/versioned/fake" @@ -30,7 +33,12 @@ func getProxy(kubeClient kubernetes.Interface) (*envoy.Proxy, error) { tests.SelectorKey: tests.BookbuyerService.Name, constants.EnvoyUniqueIDLabelName: tests.ProxyUUID, } - if _, err := tests.MakePod(kubeClient, tests.Namespace, tests.BookbuyerServiceName, tests.BookbuyerServiceAccountName, podLabels); err != nil { + + newPod1 := tests.NewPodFixture(tests.Namespace, tests.BookbuyerServiceName, tests.BookbuyerServiceAccountName, podLabels) + newPod1.Annotations = map[string]string{ + constants.PrometheusScrapeAnnotation: "true", + } + if _, err := kubeClient.CoreV1().Pods(tests.Namespace).Create(context.TODO(), &newPod1, metav1.CreateOptions{}); err != nil { return nil, err } @@ -66,7 +74,6 @@ func TestNewResponse(t *testing.T) { proxy, err := getProxy(kubeClient) assert.Empty(err) - assert.NotNil(meshCatalog) assert.NotNil(proxy) proxyRegistry := registry.NewProxyRegistry(registry.ExplicitProxyServiceMapper(func(*envoy.Proxy) ([]service.MeshService, error) { @@ -74,12 +81,15 @@ func TestNewResponse(t *testing.T) { })) mockConfigurator.EXPECT().IsPermissiveTrafficPolicyMode().Return(false).AnyTimes() - mockConfigurator.EXPECT().IsPrometheusScrapingEnabled().Return(true).AnyTimes() mockConfigurator.EXPECT().IsTracingEnabled().Return(false).AnyTimes() mockConfigurator.EXPECT().IsEgressEnabled().Return(true).AnyTimes() mockConfigurator.EXPECT().GetInboundExternalAuthConfig().Return(auth.ExtAuthConfig{ Enable: false, }).AnyTimes() + mockConfigurator.EXPECT().GetFeatureFlags().Return(v1alpha1.FeatureFlags{ + EnableWASMStats: false, + EnableEgressPolicy: true, + }).AnyTimes() resources, err := NewResponse(meshCatalog, proxy, nil, mockConfigurator, nil, proxyRegistry) assert.Empty(err) diff --git a/pkg/envoy/rds/response.go b/pkg/envoy/rds/response.go index 6dd2aff088..d60d068854 100644 --- a/pkg/envoy/rds/response.go +++ b/pkg/envoy/rds/response.go @@ -38,7 +38,7 @@ func NewResponse(cataloger catalog.MeshCataloger, proxy *envoy.Proxy, discoveryR inboundTrafficPolicies = cataloger.ListInboundTrafficPolicies(proxyIdentity.ToServiceIdentity(), services) outboundTrafficPolicies = cataloger.ListOutboundTrafficPolicies(proxyIdentity.ToServiceIdentity()) - routeConfiguration := route.BuildRouteConfiguration(inboundTrafficPolicies, outboundTrafficPolicies, proxy) + routeConfiguration := route.BuildRouteConfiguration(inboundTrafficPolicies, outboundTrafficPolicies, proxy, cfg) var rdsResources []types.Resource for _, config := range routeConfiguration { @@ -55,7 +55,7 @@ func NewResponse(cataloger catalog.MeshCataloger, proxy *envoy.Proxy, discoveryR ingressTrafficPolicies = trafficpolicy.MergeInboundPolicies(catalog.AllowPartialHostnamesMatch, ingressTrafficPolicies, ingressInboundPolicies...) } if len(ingressTrafficPolicies) > 0 { - ingressRouteConfig := route.BuildIngressConfiguration(ingressTrafficPolicies, proxy) + ingressRouteConfig := route.BuildIngressConfiguration(ingressTrafficPolicies, proxy, cfg) rdsResources = append(rdsResources, ingressRouteConfig) } diff --git a/pkg/envoy/rds/response_test.go b/pkg/envoy/rds/response_test.go index 750fdaaa87..2466610430 100644 --- a/pkg/envoy/rds/response_test.go +++ b/pkg/envoy/rds/response_test.go @@ -19,6 +19,7 @@ import ( "k8s.io/client-go/kubernetes" testclient "k8s.io/client-go/kubernetes/fake" + "github.com/openservicemesh/osm/pkg/apis/config/v1alpha1" "github.com/openservicemesh/osm/pkg/catalog" "github.com/openservicemesh/osm/pkg/certificate" "github.com/openservicemesh/osm/pkg/configurator" @@ -276,6 +277,10 @@ func TestNewResponse(t *testing.T) { mockConfigurator.EXPECT().IsPermissiveTrafficPolicyMode().Return(false).AnyTimes() + mockConfigurator.EXPECT().GetFeatureFlags().Return(v1alpha1.FeatureFlags{ + EnableWASMStats: false, + }).AnyTimes() + mockCatalog.EXPECT().ListInboundTrafficPolicies(gomock.Any(), gomock.Any()).Return(tc.expectedInboundPolicies).AnyTimes() mockCatalog.EXPECT().ListOutboundTrafficPolicies(gomock.Any()).Return(tc.expectedOutboundPolicies).AnyTimes() mockCatalog.EXPECT().GetIngressPoliciesForService(gomock.Any()).Return(tc.ingressInboundPolicies, nil).AnyTimes() @@ -530,6 +535,10 @@ func TestNewResponseWithPermissiveMode(t *testing.T) { mockConfigurator.EXPECT().IsPermissiveTrafficPolicyMode().Return(true).AnyTimes() + mockConfigurator.EXPECT().GetFeatureFlags().Return(v1alpha1.FeatureFlags{ + EnableWASMStats: false, + }).AnyTimes() + resources, err := NewResponse(mockCatalog, testProxy, &discoveryRequest, mockConfigurator, nil, proxyRegistry) assert.Nil(err) @@ -593,6 +602,9 @@ func TestResponseRequestCompletion(t *testing.T) { mockCatalog.EXPECT().GetIngressPoliciesForService(gomock.Any()).Return([]*trafficpolicy.InboundTrafficPolicy{}, nil).AnyTimes() mockCatalog.EXPECT().GetEgressTrafficPolicy(gomock.Any()).Return(nil, nil).AnyTimes() mockConfigurator.EXPECT().IsPermissiveTrafficPolicyMode().Return(false).AnyTimes() + mockConfigurator.EXPECT().GetFeatureFlags().Return(v1alpha1.FeatureFlags{ + EnableWASMStats: false, + }).AnyTimes() testCases := []struct { request *xds_discovery.DiscoveryRequest diff --git a/pkg/envoy/rds/route/route_config.go b/pkg/envoy/rds/route/route_config.go index d19c3725a9..f008c63044 100644 --- a/pkg/envoy/rds/route/route_config.go +++ b/pkg/envoy/rds/route/route_config.go @@ -10,9 +10,9 @@ import ( xds_matcher "github.com/envoyproxy/go-control-plane/envoy/type/matcher/v3" "github.com/golang/protobuf/ptypes/wrappers" + "github.com/openservicemesh/osm/pkg/configurator" "github.com/openservicemesh/osm/pkg/constants" "github.com/openservicemesh/osm/pkg/envoy" - "github.com/openservicemesh/osm/pkg/featureflags" "github.com/openservicemesh/osm/pkg/service" "github.com/openservicemesh/osm/pkg/trafficpolicy" ) @@ -64,7 +64,7 @@ const ( ) // BuildRouteConfiguration constructs the Envoy constructs ([]*xds_route.RouteConfiguration) for implementing inbound and outbound routes -func BuildRouteConfiguration(inbound []*trafficpolicy.InboundTrafficPolicy, outbound []*trafficpolicy.OutboundTrafficPolicy, proxy *envoy.Proxy) []*xds_route.RouteConfiguration { +func BuildRouteConfiguration(inbound []*trafficpolicy.InboundTrafficPolicy, outbound []*trafficpolicy.OutboundTrafficPolicy, proxy *envoy.Proxy, cfg configurator.Configurator) []*xds_route.RouteConfiguration { var routeConfiguration []*xds_route.RouteConfiguration // For both Inbound and Outbound routes, we will always generate the route resource stubs and send them even when empty, @@ -77,7 +77,7 @@ func BuildRouteConfiguration(inbound []*trafficpolicy.InboundTrafficPolicy, outb inboundRouteConfig.VirtualHosts = append(inboundRouteConfig.VirtualHosts, virtualHost) } - if featureflags.IsWASMStatsEnabled() { + if featureFlags := cfg.GetFeatureFlags(); featureFlags.EnableWASMStats { for k, v := range proxy.StatsHeaders() { inboundRouteConfig.ResponseHeadersToAdd = append(inboundRouteConfig.ResponseHeadersToAdd, &core.HeaderValueOption{ Header: &core.HeaderValue{ @@ -102,7 +102,7 @@ func BuildRouteConfiguration(inbound []*trafficpolicy.InboundTrafficPolicy, outb } // BuildIngressConfiguration constructs the Envoy constructs ([]*xds_route.RouteConfiguration) for implementing ingress routes -func BuildIngressConfiguration(ingress []*trafficpolicy.InboundTrafficPolicy, proxy *envoy.Proxy) *xds_route.RouteConfiguration { +func BuildIngressConfiguration(ingress []*trafficpolicy.InboundTrafficPolicy, proxy *envoy.Proxy, cfg configurator.Configurator) *xds_route.RouteConfiguration { if len(ingress) == 0 { return nil } @@ -114,7 +114,7 @@ func BuildIngressConfiguration(ingress []*trafficpolicy.InboundTrafficPolicy, pr ingressRouteConfig.VirtualHosts = append(ingressRouteConfig.VirtualHosts, virtualHost) } - if featureflags.IsWASMStatsEnabled() { + if featureFlags := cfg.GetFeatureFlags(); featureFlags.EnableWASMStats { for k, v := range proxy.StatsHeaders() { ingressRouteConfig.ResponseHeadersToAdd = append(ingressRouteConfig.ResponseHeadersToAdd, &core.HeaderValueOption{ Header: &core.HeaderValue{ diff --git a/pkg/envoy/rds/route/route_config_test.go b/pkg/envoy/rds/route/route_config_test.go index 3eac65f49e..5411c5e6ed 100644 --- a/pkg/envoy/rds/route/route_config_test.go +++ b/pkg/envoy/rds/route/route_config_test.go @@ -7,12 +7,14 @@ import ( mapset "github.com/deckarep/golang-set" xds_route "github.com/envoyproxy/go-control-plane/envoy/config/route/v3" xds_matcher "github.com/envoyproxy/go-control-plane/envoy/type/matcher/v3" + "github.com/golang/mock/gomock" "github.com/golang/protobuf/ptypes/wrappers" tassert "github.com/stretchr/testify/assert" + "github.com/openservicemesh/osm/pkg/apis/config/v1alpha1" + "github.com/openservicemesh/osm/pkg/configurator" "github.com/openservicemesh/osm/pkg/constants" "github.com/openservicemesh/osm/pkg/envoy" - "github.com/openservicemesh/osm/pkg/featureflags" "github.com/openservicemesh/osm/pkg/identity" "github.com/openservicemesh/osm/pkg/service" "github.com/openservicemesh/osm/pkg/tests" @@ -21,6 +23,10 @@ import ( func TestBuildRouteConfiguration(t *testing.T) { assert := tassert.New(t) + + mockCtrl := gomock.NewController(t) + mockCfg := configurator.NewMockConfigurator(mockCtrl) + testInbound := &trafficpolicy.InboundTrafficPolicy{ Name: "bookstore-v1-default", Hostnames: tests.BookstoreV1Hostnames, @@ -90,7 +96,10 @@ func TestBuildRouteConfiguration(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - actual := BuildRouteConfiguration(tc.inbound, tc.outbound, nil) + mockCfg.EXPECT().GetFeatureFlags().Return(v1alpha1.FeatureFlags{ + EnableWASMStats: false, + }).Times(1) + actual := BuildRouteConfiguration(tc.inbound, tc.outbound, nil, mockCfg) assert.Equal(tc.expectedRouteConfigLen, len(actual)) }) } @@ -114,14 +123,12 @@ func TestBuildRouteConfiguration(t *testing.T) { for _, tc := range statsWASMTestCases { t.Run(tc.name, func(t *testing.T) { - oldWASMflag := featureflags.IsWASMStatsEnabled() - featureflags.Features.WASMStats = tc.wasmEnabled - - actual := BuildRouteConfiguration([]*trafficpolicy.InboundTrafficPolicy{testInbound}, nil, &envoy.Proxy{}) + mockCfg.EXPECT().GetFeatureFlags().Return(v1alpha1.FeatureFlags{ + EnableWASMStats: tc.wasmEnabled, + }).Times(1) + actual := BuildRouteConfiguration([]*trafficpolicy.InboundTrafficPolicy{testInbound}, nil, &envoy.Proxy{}, mockCfg) tassert.Len(t, actual, 2) tassert.Len(t, actual[0].ResponseHeadersToAdd, tc.expectedResponseHeaderLen) - - featureflags.Features.WASMStats = oldWASMflag }) } } @@ -129,6 +136,9 @@ func TestBuildRouteConfiguration(t *testing.T) { func TestBuildIngressRouteConfiguration(t *testing.T) { assert := tassert.New(t) + mockCtrl := gomock.NewController(t) + mockCfg := configurator.NewMockConfigurator(mockCtrl) + testCases := []struct { name string ingressPolicies []*trafficpolicy.InboundTrafficPolicy @@ -205,7 +215,10 @@ func TestBuildIngressRouteConfiguration(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - actual := BuildIngressConfiguration(tc.ingressPolicies, nil) + mockCfg.EXPECT().GetFeatureFlags().Return(v1alpha1.FeatureFlags{ + EnableWASMStats: false, + }).AnyTimes() + actual := BuildIngressConfiguration(tc.ingressPolicies, nil, mockCfg) if tc.expectedRouteConfigFields == nil { assert.Nil(actual) diff --git a/pkg/envoy/xdsutil.go b/pkg/envoy/xdsutil.go index 31044436f3..b1a9526fa1 100644 --- a/pkg/envoy/xdsutil.go +++ b/pkg/envoy/xdsutil.go @@ -1,7 +1,6 @@ package envoy import ( - "errors" "fmt" "strings" @@ -16,6 +15,7 @@ import ( structpb "github.com/golang/protobuf/ptypes/struct" "github.com/golang/protobuf/ptypes/wrappers" "github.com/google/uuid" + "github.com/pkg/errors" v1 "k8s.io/api/core/v1" "github.com/openservicemesh/osm/pkg/certificate" diff --git a/pkg/featureflags/featureflags.go b/pkg/featureflags/featureflags.go deleted file mode 100644 index a50cf58252..0000000000 --- a/pkg/featureflags/featureflags.go +++ /dev/null @@ -1,49 +0,0 @@ -// Package featureflags implements routines to check if a given feature is enabled. -package featureflags - -import ( - "sync" -) - -// OptionalFeatures is a struct to enable/disable optional features -type OptionalFeatures struct { - WASMStats bool - EgressPolicy bool - MulticlusterMode bool -} - -var ( - // Features describes whether an optional feature is enabled - Features OptionalFeatures - - once sync.Once -) - -// Initialize initializes the feature flag options -func Initialize(optionalFeatures OptionalFeatures) { - once.Do(func() { - Features = optionalFeatures - }) -} - -/* Feature flag stub -// IsFeatureNameEnabled returns a boolean indicating if the feature `FeatureName` is enabled -func IsFeatureNameEnabled() bool { - return Features.FeatureName -} -*/ - -// IsWASMStatsEnabled returns a boolean indicating if custom stats will be generated by a WASM extension to Envoy -func IsWASMStatsEnabled() bool { - return Features.WASMStats -} - -// IsEgressPolicyEnabled returns a boolean indicating if OSM's Egress policy API is enabled -func IsEgressPolicyEnabled() bool { - return Features.EgressPolicy -} - -// IsMulticlusterModeEnabled returns a boolean indicating if multicluster mode is enabled in OSM -func IsMulticlusterModeEnabled() bool { - return Features.MulticlusterMode -} diff --git a/pkg/featureflags/featureflags_test.go b/pkg/featureflags/featureflags_test.go deleted file mode 100644 index c2d10d6bd8..0000000000 --- a/pkg/featureflags/featureflags_test.go +++ /dev/null @@ -1,38 +0,0 @@ -package featureflags - -import ( - "testing" - - tassert "github.com/stretchr/testify/assert" -) - -func TestFlags(t *testing.T) { - assert := tassert.New(t) - - // 1. Verify all optional features are disabled by default - assert.Equal(false, IsWASMStatsEnabled()) - assert.Equal(false, IsEgressPolicyEnabled()) - assert.Equal(false, IsMulticlusterModeEnabled()) - - // 2. Enable all optional features and verify they are enabled - optionalFeatures := OptionalFeatures{ - WASMStats: true, - EgressPolicy: true, - MulticlusterMode: true, - } - Initialize(optionalFeatures) - assert.Equal(true, IsWASMStatsEnabled()) - assert.Equal(true, IsEgressPolicyEnabled()) - assert.Equal(true, IsMulticlusterModeEnabled()) - - // 3. Verify features cannot be reinitialized - optionalFeatures = OptionalFeatures{ - WASMStats: false, - EgressPolicy: false, - MulticlusterMode: false, - } - Initialize(optionalFeatures) - assert.Equal(true, IsWASMStatsEnabled()) - assert.Equal(true, IsEgressPolicyEnabled()) - assert.Equal(true, IsMulticlusterModeEnabled()) -} diff --git a/pkg/featureflags/suite_test.go b/pkg/featureflags/suite_test.go deleted file mode 100644 index f217f86fe6..0000000000 --- a/pkg/featureflags/suite_test.go +++ /dev/null @@ -1,13 +0,0 @@ -package featureflags - -import ( - "testing" - - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" -) - -func TestFeatureFlags(t *testing.T) { - RegisterFailHandler(Fail) - RunSpecs(t, "Feature flags Test Suite") -} diff --git a/pkg/kubernetes/client.go b/pkg/kubernetes/client.go index 717115f745..53a3633103 100644 --- a/pkg/kubernetes/client.go +++ b/pkg/kubernetes/client.go @@ -2,6 +2,7 @@ package kubernetes import ( "reflect" + "strconv" mapset "github.com/deckarep/golang-set" "github.com/pkg/errors" @@ -298,3 +299,15 @@ func (c Client) ListServiceIdentitiesForService(svc service.MeshService) ([]iden } return svcAccounts, nil } + +// IsMetricsEnabled returns true if the pod in the mesh is correctly annotated for prometheus scrapping +func (c Client) IsMetricsEnabled(pod *corev1.Pod) bool { + isScrapingEnabled := false + prometheusScrapeAnnotation, ok := pod.Annotations[constants.PrometheusScrapeAnnotation] + if !ok { + return isScrapingEnabled + } + + isScrapingEnabled, _ = strconv.ParseBool(prometheusScrapeAnnotation) + return isScrapingEnabled +} diff --git a/pkg/kubernetes/client_test.go b/pkg/kubernetes/client_test.go index 553369f6c7..4fb96121d0 100644 --- a/pkg/kubernetes/client_test.go +++ b/pkg/kubernetes/client_test.go @@ -3,12 +3,14 @@ package kubernetes import ( "context" "fmt" + "testing" "time" mapset "github.com/deckarep/golang-set" "github.com/google/uuid" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + tassert "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" testclient "k8s.io/client-go/kubernetes/fake" @@ -553,3 +555,79 @@ var _ = Describe("Test Namespace KubeController Methods", func() { }) }) + +func TestIsMetricsEnabled(t *testing.T) { + assert := tassert.New(t) + + testCases := []struct { + name string + addPrometheusAnnotation bool + expectedMetricsScraping bool + scrapingAnnotation string + }{ + { + name: "pod without prometheus scraping annotation", + scrapingAnnotation: "false", + addPrometheusAnnotation: false, + expectedMetricsScraping: false, + }, + { + name: "pod with prometheus scraping annotation set to true", + scrapingAnnotation: "true", + addPrometheusAnnotation: true, + expectedMetricsScraping: true, + }, + { + name: "pod with prometheus scraping annotation set to false", + scrapingAnnotation: "false", + addPrometheusAnnotation: true, + expectedMetricsScraping: false, + }, + { + name: "pod with incorrect prometheus scraping annotation", + scrapingAnnotation: "no", + addPrometheusAnnotation: true, + expectedMetricsScraping: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + kubeClient := testclient.NewSimpleClientset() + stop := make(chan struct{}) + kubeController, err := NewKubernetesController(kubeClient, testMeshName, stop) + assert.Nil(err) + assert.NotNil(kubeController) + + proxyUUID := uuid.New() + namespace := uuid.New().String() + podlabels := map[string]string{ + tests.SelectorKey: tests.SelectorValue, + constants.EnvoyUniqueIDLabelName: proxyUUID.String(), + } + + // Ensure correct presetup + pods, err := kubeClient.CoreV1().Pods(namespace).List(context.TODO(), metav1.ListOptions{}) + assert.Nil(err) + assert.Len(pods.Items, 0) + + newPod1 := tests.NewPodFixture(namespace, fmt.Sprintf("pod-1-%s", proxyUUID), tests.BookstoreServiceAccountName, podlabels) + + if tc.addPrometheusAnnotation { + newPod1.Annotations = map[string]string{ + constants.PrometheusScrapeAnnotation: tc.scrapingAnnotation, + } + } + _, err = kubeClient.CoreV1().Pods(namespace).Create(context.TODO(), &newPod1, metav1.CreateOptions{}) + assert.Nil(err) + + // Ensure correct setup + pods, err = kubeClient.CoreV1().Pods(namespace).List(context.TODO(), metav1.ListOptions{}) + assert.Nil(err) + assert.Len(pods.Items, 1) + + actual := kubeController.IsMetricsEnabled(&newPod1) + assert.Equal(actual, tc.expectedMetricsScraping) + }) + } +} diff --git a/pkg/kubernetes/mock_controller_generated.go b/pkg/kubernetes/mock_controller_generated.go index 0b0c2b5276..eae6311970 100644 --- a/pkg/kubernetes/mock_controller_generated.go +++ b/pkg/kubernetes/mock_controller_generated.go @@ -79,6 +79,20 @@ func (mr *MockControllerMockRecorder) GetService(arg0 interface{}) *gomock.Call return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetService", reflect.TypeOf((*MockController)(nil).GetService), arg0) } +// IsMetricsEnabled mocks base method +func (m *MockController) IsMetricsEnabled(arg0 *v1.Pod) bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "IsMetricsEnabled", arg0) + ret0, _ := ret[0].(bool) + return ret0 +} + +// IsMetricsEnabled indicates an expected call of IsMetricsEnabled +func (mr *MockControllerMockRecorder) IsMetricsEnabled(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsMetricsEnabled", reflect.TypeOf((*MockController)(nil).IsMetricsEnabled), arg0) +} + // IsMonitoredNamespace mocks base method func (m *MockController) IsMonitoredNamespace(arg0 string) bool { m.ctrl.T.Helper() diff --git a/pkg/kubernetes/types.go b/pkg/kubernetes/types.go index d5227b5e4e..a839b6bfbb 100644 --- a/pkg/kubernetes/types.go +++ b/pkg/kubernetes/types.go @@ -100,4 +100,7 @@ type Controller interface { // GetEndpoints returns the endpoints for a given service, if found GetEndpoints(svc service.MeshService) (*corev1.Endpoints, error) + + // IsMetricsEnabled returns true if the pod in the mesh is correctly annotated for prometheus scrapping + IsMetricsEnabled(*corev1.Pod) bool } diff --git a/tests/e2e/e2e_helm_install_test.go b/tests/e2e/e2e_helm_install_test.go index dfc63791f2..fe597606b0 100644 --- a/tests/e2e/e2e_helm_install_test.go +++ b/tests/e2e/e2e_helm_install_test.go @@ -34,7 +34,6 @@ var _ = OSMDescribe("Test osm control plane installation with Helm", Expect(spec.Traffic.EnableEgress).To(BeFalse()) Expect(spec.Sidecar.LogLevel).To(Equal("error")) Expect(spec.Observability.EnableDebugServer).To(BeFalse()) - Expect(spec.Observability.PrometheusScraping).To(BeTrue()) Expect(spec.Observability.Tracing.Enable).To(BeFalse()) Expect(spec.Traffic.UseHTTPSIngress).To(BeFalse()) Expect(spec.Certificate.ServiceCertValidityDuration).To(Equal("24h")) diff --git a/tests/e2e/e2e_init_controller_test.go b/tests/e2e/e2e_init_controller_test.go index fcdcd8ed8d..b57198a712 100644 --- a/tests/e2e/e2e_init_controller_test.go +++ b/tests/e2e/e2e_init_controller_test.go @@ -30,7 +30,6 @@ var _ = OSMDescribe("Test init-osm-controller functionalities", Expect(meshConfig.Spec.Traffic.EnablePermissiveTrafficPolicyMode).Should(BeFalse()) Expect(meshConfig.Spec.Traffic.EnableEgress).Should(BeFalse()) Expect(meshConfig.Spec.Sidecar.LogLevel).Should(Equal("debug")) - Expect(meshConfig.Spec.Observability.PrometheusScraping).Should(BeTrue()) Expect(meshConfig.Spec.Observability.EnableDebugServer).Should(BeTrue()) Expect(meshConfig.Spec.Observability.Tracing.Enable).Should(BeFalse()) Expect(meshConfig.Spec.Traffic.UseHTTPSIngress).Should(BeFalse()) diff --git a/tests/scenarios/traffic_split_with_apex_service_test.go b/tests/scenarios/traffic_split_with_apex_service_test.go index 2c1eb19b13..86144f4502 100644 --- a/tests/scenarios/traffic_split_with_apex_service_test.go +++ b/tests/scenarios/traffic_split_with_apex_service_test.go @@ -14,6 +14,7 @@ import ( "k8s.io/client-go/kubernetes" testclient "k8s.io/client-go/kubernetes/fake" + "github.com/openservicemesh/osm/pkg/apis/config/v1alpha1" configFake "github.com/openservicemesh/osm/pkg/gen/client/config/clientset/versioned/fake" "github.com/openservicemesh/osm/pkg/catalog" @@ -56,6 +57,11 @@ var _ = Describe(``+ // ---[ Get the config from rds.NewResponse() ]------- mockConfigurator.EXPECT().IsPermissiveTrafficPolicyMode().Return(false).AnyTimes() + mockConfigurator.EXPECT().GetFeatureFlags().Return(v1alpha1.FeatureFlags{ + EnableWASMStats: false, + EnableEgressPolicy: false, + }).AnyTimes() + resources, err := rds.NewResponse(meshCatalog, proxy, nil, mockConfigurator, nil, proxyRegistry) It("did not return an error", func() { Expect(err).ToNot(HaveOccurred()) diff --git a/tests/scenarios/traffic_split_with_zero_weight_test.go b/tests/scenarios/traffic_split_with_zero_weight_test.go index c4ea2d9d34..70abdd4efa 100644 --- a/tests/scenarios/traffic_split_with_zero_weight_test.go +++ b/tests/scenarios/traffic_split_with_zero_weight_test.go @@ -17,6 +17,7 @@ import ( "k8s.io/client-go/kubernetes" testclient "k8s.io/client-go/kubernetes/fake" + "github.com/openservicemesh/osm/pkg/apis/config/v1alpha1" "github.com/openservicemesh/osm/pkg/catalog" "github.com/openservicemesh/osm/pkg/certificate" "github.com/openservicemesh/osm/pkg/configurator" @@ -234,6 +235,10 @@ func TestRDSRespose(t *testing.T) { mockConfigurator.EXPECT().IsPermissiveTrafficPolicyMode().Return(false).AnyTimes() + mockConfigurator.EXPECT().GetFeatureFlags().Return(v1alpha1.FeatureFlags{ + EnableWASMStats: false, + }).AnyTimes() + proxyRegistry := registry.NewProxyRegistry(registry.ExplicitProxyServiceMapper(func(*envoy.Proxy) ([]service.MeshService, error) { return []service.MeshService{tests.BookstoreV1Service}, nil }))