From 532be03850504d71b70aea78e4eef479d94fbb4f Mon Sep 17 00:00:00 2001 From: Periklis Tsirakidis Date: Tue, 16 Jul 2024 12:30:01 +0200 Subject: [PATCH] fix(operator): Allow structured metadata only if V13 schema provided (#13463) Co-authored-by: Robert Jacob --- operator/CHANGELOG.md | 1 + .../handlers/internal/storage/storage.go | 17 ++- .../handlers/internal/storage/storage_test.go | 122 ++++++++++++++++++ .../manifests/internal/config/build_test.go | 62 +++++---- .../internal/config/loki-config.yaml | 2 +- .../internal/manifests/storage/options.go | 7 +- 6 files changed, 182 insertions(+), 29 deletions(-) diff --git a/operator/CHANGELOG.md b/operator/CHANGELOG.md index 96f1d59a5e1f..8efad7020b70 100644 --- a/operator/CHANGELOG.md +++ b/operator/CHANGELOG.md @@ -2,6 +2,7 @@ ## Release 5.9.4 +- [13463](https://github.com/grafana/loki/pull/13463) **periklis**: fix(operator): Allow structured metadata only if V13 schema provided - [13450](https://github.com/grafana/loki/pull/13450) **periklis**: fix(operator): Skip updating annotations for serviceaccounts - [13430](https://github.com/grafana/loki/pull/13430) **periklis**: fix(operator): Support v3.1.0 in OpenShift dashboards - [13422](https://github.com/grafana/loki/pull/13422) **periklis** feat(operator): Update Loki operand to v3.1.0 diff --git a/operator/internal/handlers/internal/storage/storage.go b/operator/internal/handlers/internal/storage/storage.go index b2d40d6cf32c..6276942fe32e 100644 --- a/operator/internal/handlers/internal/storage/storage.go +++ b/operator/internal/handlers/internal/storage/storage.go @@ -45,8 +45,9 @@ func BuildOptions(ctx context.Context, k k8s.Client, stack *lokiv1.LokiStack, fg } } + now := time.Now().UTC() storageSchemas, err := storage.BuildSchemaConfig( - time.Now().UTC(), + now, stack.Spec.Storage, stack.Status.Storage, ) @@ -59,6 +60,7 @@ func BuildOptions(ctx context.Context, k k8s.Client, stack *lokiv1.LokiStack, fg } objStore.Schemas = storageSchemas + objStore.AllowStructuredMetadata = allowStructuredMetadata(storageSchemas, now) if stack.Spec.Storage.TLS == nil { return objStore, nil @@ -98,3 +100,16 @@ func BuildOptions(ctx context.Context, k k8s.Client, stack *lokiv1.LokiStack, fg return objStore, nil } + +func allowStructuredMetadata(schemas []lokiv1.ObjectStorageSchema, now time.Time) bool { + activeVersion := lokiv1.ObjectStorageSchemaV11 + for _, s := range schemas { + time, _ := s.EffectiveDate.UTCTime() + if time.Before(now) { + activeVersion = s.Version + } + } + + return activeVersion != lokiv1.ObjectStorageSchemaV11 && + activeVersion != lokiv1.ObjectStorageSchemaV12 +} diff --git a/operator/internal/handlers/internal/storage/storage_test.go b/operator/internal/handlers/internal/storage/storage_test.go index 857d0be4de4a..05cfd88bb5ff 100644 --- a/operator/internal/handlers/internal/storage/storage_test.go +++ b/operator/internal/handlers/internal/storage/storage_test.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "testing" + "time" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" @@ -560,3 +561,124 @@ func TestBuildOptions_WhenInvalidCAConfigMap_SetDegraded(t *testing.T) { require.Error(t, err) require.Equal(t, degradedErr, err) } + +func TestAllowStructuredMetadata(t *testing.T) { + testTime := time.Date(2024, 7, 1, 1, 0, 0, 0, time.UTC) + tt := []struct { + desc string + schemas []lokiv1.ObjectStorageSchema + wantAllow bool + }{ + { + desc: "disallow - no schemas", + schemas: []lokiv1.ObjectStorageSchema{}, + wantAllow: false, + }, + { + desc: "disallow - only v12", + schemas: []lokiv1.ObjectStorageSchema{ + { + Version: lokiv1.ObjectStorageSchemaV12, + EffectiveDate: "2024-07-01", + }, + }, + wantAllow: false, + }, + { + desc: "allow - only v13", + schemas: []lokiv1.ObjectStorageSchema{ + { + Version: lokiv1.ObjectStorageSchemaV13, + EffectiveDate: "2024-07-01", + }, + }, + wantAllow: true, + }, + { + desc: "disallow - v13 in future", + schemas: []lokiv1.ObjectStorageSchema{ + { + Version: lokiv1.ObjectStorageSchemaV12, + EffectiveDate: "2024-07-01", + }, + { + Version: lokiv1.ObjectStorageSchemaV13, + EffectiveDate: "2024-07-02", + }, + }, + wantAllow: false, + }, + { + desc: "disallow - v13 in past", + schemas: []lokiv1.ObjectStorageSchema{ + { + Version: lokiv1.ObjectStorageSchemaV13, + EffectiveDate: "2024-06-01", + }, + { + Version: lokiv1.ObjectStorageSchemaV12, + EffectiveDate: "2024-07-01", + }, + }, + wantAllow: false, + }, + { + desc: "disallow - v13 in past and future", + schemas: []lokiv1.ObjectStorageSchema{ + { + Version: lokiv1.ObjectStorageSchemaV13, + EffectiveDate: "2024-06-01", + }, + { + Version: lokiv1.ObjectStorageSchemaV12, + EffectiveDate: "2024-07-01", + }, + { + Version: lokiv1.ObjectStorageSchemaV13, + EffectiveDate: "2024-07-02", + }, + }, + wantAllow: false, + }, + { + desc: "allow - v13 active", + schemas: []lokiv1.ObjectStorageSchema{ + { + Version: lokiv1.ObjectStorageSchemaV12, + EffectiveDate: "2024-06-01", + }, + { + Version: lokiv1.ObjectStorageSchemaV13, + EffectiveDate: "2024-07-01", + }, + }, + wantAllow: true, + }, + { + desc: "allow - v13 active, v12 in future", + schemas: []lokiv1.ObjectStorageSchema{ + { + Version: lokiv1.ObjectStorageSchemaV13, + EffectiveDate: "2024-07-01", + }, + { + Version: lokiv1.ObjectStorageSchemaV12, + EffectiveDate: "2024-08-01", + }, + }, + wantAllow: true, + }, + } + + for _, tc := range tt { + tc := tc + t.Run(tc.desc, func(t *testing.T) { + t.Parallel() + + allow := allowStructuredMetadata(tc.schemas, testTime) + if allow != tc.wantAllow { + t.Errorf("got %v, want %v", allow, tc.wantAllow) + } + }) + } +} diff --git a/operator/internal/manifests/internal/config/build_test.go b/operator/internal/manifests/internal/config/build_test.go index fb291157cab8..d21046749884 100644 --- a/operator/internal/manifests/internal/config/build_test.go +++ b/operator/internal/manifests/internal/config/build_test.go @@ -114,7 +114,7 @@ limits_config: shard_streams: enabled: true desired_rate: 3MB - allow_structured_metadata: true + allow_structured_metadata: false memberlist: abort_if_cluster_join_fails: true advertise_port: 7946 @@ -371,7 +371,7 @@ limits_config: shard_streams: enabled: true desired_rate: 3MB - allow_structured_metadata: true + allow_structured_metadata: false memberlist: abort_if_cluster_join_fails: true advertise_port: 7946 @@ -797,7 +797,7 @@ limits_config: shard_streams: enabled: true desired_rate: 3MB - allow_structured_metadata: true + allow_structured_metadata: false memberlist: abort_if_cluster_join_fails: true advertise_port: 7946 @@ -1155,7 +1155,7 @@ limits_config: shard_streams: enabled: true desired_rate: 3MB - allow_structured_metadata: true + allow_structured_metadata: false memberlist: abort_if_cluster_join_fails: true advertise_port: 7946 @@ -1514,7 +1514,7 @@ limits_config: shard_streams: enabled: true desired_rate: 3MB - allow_structured_metadata: true + allow_structured_metadata: false memberlist: abort_if_cluster_join_fails: true advertise_port: 7946 @@ -1911,7 +1911,7 @@ limits_config: shard_streams: enabled: true desired_rate: 3MB - allow_structured_metadata: true + allow_structured_metadata: false memberlist: abort_if_cluster_join_fails: true advertise_port: 7946 @@ -2241,7 +2241,7 @@ limits_config: shard_streams: enabled: true desired_rate: 3MB - allow_structured_metadata: true + allow_structured_metadata: false memberlist: abort_if_cluster_join_fails: true advertise_port: 7946 @@ -2681,7 +2681,7 @@ limits_config: shard_streams: enabled: true desired_rate: 3MB - allow_structured_metadata: true + allow_structured_metadata: false memberlist: abort_if_cluster_join_fails: true advertise_port: 7946 @@ -3006,7 +3006,7 @@ limits_config: shard_streams: enabled: true desired_rate: 3MB - allow_structured_metadata: true + allow_structured_metadata: false memberlist: abort_if_cluster_join_fails: true advertise_port: 7946 @@ -3503,7 +3503,7 @@ limits_config: shard_streams: enabled: true desired_rate: 3MB - allow_structured_metadata: true + allow_structured_metadata: false memberlist: abort_if_cluster_join_fails: true advertise_addr: ${HASH_RING_INSTANCE_ADDR} @@ -3764,7 +3764,7 @@ limits_config: shard_streams: enabled: true desired_rate: 3MB - allow_structured_metadata: true + allow_structured_metadata: false memberlist: abort_if_cluster_join_fails: true advertise_addr: ${HASH_RING_INSTANCE_ADDR} @@ -4026,7 +4026,7 @@ limits_config: shard_streams: enabled: true desired_rate: 3MB - allow_structured_metadata: true + allow_structured_metadata: false memberlist: abort_if_cluster_join_fails: true advertise_port: 7946 @@ -4289,7 +4289,7 @@ limits_config: shard_streams: enabled: true desired_rate: 3MB - allow_structured_metadata: true + allow_structured_metadata: false memberlist: abort_if_cluster_join_fails: true advertise_port: 7946 @@ -4588,7 +4588,7 @@ limits_config: shard_streams: enabled: true desired_rate: 3MB - allow_structured_metadata: true + allow_structured_metadata: false memberlist: abort_if_cluster_join_fails: true advertise_port: 7946 @@ -4882,7 +4882,7 @@ limits_config: query_timeout: 1m volume_enabled: true volume_max_series: 1000 - allow_structured_metadata: true + allow_structured_metadata: false memberlist: abort_if_cluster_join_fails: true advertise_port: 7946 @@ -5126,11 +5126,13 @@ func defaultOptions() Options { func TestBuild_ConfigAndRuntimeConfig_Schemas(t *testing.T) { for _, tc := range []struct { - name string - schemaConfig []lokiv1.ObjectStorageSchema - shippers []string - expSchemaConfig string - expStorageConfig string + name string + schemaConfig []lokiv1.ObjectStorageSchema + shippers []string + allowStructuredMetadata bool + expSchemaConfig string + expStorageConfig string + expStructuredMetadata string }{ { name: "default_config_v11_schema", @@ -5158,6 +5160,8 @@ func TestBuild_ConfigAndRuntimeConfig_Schemas(t *testing.T) { resync_interval: 5m index_gateway_client: server_address: dns:///loki-index-gateway-grpc-lokistack-dev.default.svc.cluster.local:9095`, + expStructuredMetadata: ` + allow_structured_metadata: false`, }, { name: "v12_schema", @@ -5185,6 +5189,8 @@ func TestBuild_ConfigAndRuntimeConfig_Schemas(t *testing.T) { resync_interval: 5m index_gateway_client: server_address: dns:///loki-index-gateway-grpc-lokistack-dev.default.svc.cluster.local:9095`, + expStructuredMetadata: ` + allow_structured_metadata: false`, }, { name: "v13_schema", @@ -5194,7 +5200,8 @@ func TestBuild_ConfigAndRuntimeConfig_Schemas(t *testing.T) { EffectiveDate: "2024-01-01", }, }, - shippers: []string{"tsdb"}, + allowStructuredMetadata: true, + shippers: []string{"tsdb"}, expSchemaConfig: ` configs: - from: "2024-01-01" @@ -5212,6 +5219,8 @@ func TestBuild_ConfigAndRuntimeConfig_Schemas(t *testing.T) { resync_interval: 5m index_gateway_client: server_address: dns:///loki-index-gateway-grpc-lokistack-dev.default.svc.cluster.local:9095`, + expStructuredMetadata: ` + allow_structured_metadata: true`, }, { name: "multiple_schema", @@ -5229,7 +5238,8 @@ func TestBuild_ConfigAndRuntimeConfig_Schemas(t *testing.T) { EffectiveDate: "2024-01-01", }, }, - shippers: []string{"boltdb", "tsdb"}, + shippers: []string{"boltdb", "tsdb"}, + allowStructuredMetadata: true, expSchemaConfig: ` configs: - from: "2020-01-01" @@ -5268,6 +5278,8 @@ func TestBuild_ConfigAndRuntimeConfig_Schemas(t *testing.T) { resync_interval: 5m index_gateway_client: server_address: dns:///loki-index-gateway-grpc-lokistack-dev.default.svc.cluster.local:9095`, + expStructuredMetadata: ` + allow_structured_metadata: true`, }, } { t.Run(tc.name, func(t *testing.T) { @@ -5368,7 +5380,7 @@ limits_config: query_timeout: 1m volume_enabled: true volume_max_series: 1000 - allow_structured_metadata: true + ${STORAGE_STRUCTURED_METADATA} memberlist: abort_if_cluster_join_fails: true advertise_port: 7946 @@ -5418,9 +5430,11 @@ analytics: ` expCfg = strings.Replace(expCfg, "${SCHEMA_CONFIG}", tc.expSchemaConfig, 1) expCfg = strings.Replace(expCfg, "${STORAGE_CONFIG}", tc.expStorageConfig, 1) + expCfg = strings.Replace(expCfg, "${STORAGE_STRUCTURED_METADATA}", tc.expStructuredMetadata, 1) opts := defaultOptions() opts.ObjectStorage.Schemas = tc.schemaConfig + opts.ObjectStorage.AllowStructuredMetadata = tc.allowStructuredMetadata opts.Shippers = tc.shippers cfg, _, err := Build(opts) @@ -5542,7 +5556,7 @@ limits_config: query_timeout: 1m volume_enabled: true volume_max_series: 1000 - allow_structured_metadata: true + allow_structured_metadata: false memberlist: abort_if_cluster_join_fails: true advertise_port: 7946 diff --git a/operator/internal/manifests/internal/config/loki-config.yaml b/operator/internal/manifests/internal/config/loki-config.yaml index bb9bb388249e..2afd24e1e9d6 100644 --- a/operator/internal/manifests/internal/config/loki-config.yaml +++ b/operator/internal/manifests/internal/config/loki-config.yaml @@ -220,7 +220,7 @@ limits_config: enabled: true desired_rate: {{ . }}MB {{- end }} - allow_structured_metadata: true + allow_structured_metadata: {{ .ObjectStorage.AllowStructuredMetadata }} {{- with .GossipRing }} memberlist: abort_if_cluster_join_fails: true diff --git a/operator/internal/manifests/storage/options.go b/operator/internal/manifests/storage/options.go index 1f8d3d904b71..1adb80235b09 100644 --- a/operator/internal/manifests/storage/options.go +++ b/operator/internal/manifests/storage/options.go @@ -7,9 +7,10 @@ import ( // Options is used to configure Loki to integrate with // supported object storages. type Options struct { - Schemas []lokiv1.ObjectStorageSchema - SharedStore lokiv1.ObjectStorageSecretType - CredentialMode lokiv1.CredentialMode + Schemas []lokiv1.ObjectStorageSchema + SharedStore lokiv1.ObjectStorageSecretType + CredentialMode lokiv1.CredentialMode + AllowStructuredMetadata bool Azure *AzureStorageConfig GCS *GCSStorageConfig