From 4440e0d89e06897b5d9516d0eeb3e3787ba29862 Mon Sep 17 00:00:00 2001 From: bdumpp Date: Fri, 3 Jul 2026 12:58:50 +0200 Subject: [PATCH 1/4] =?UTF-8?q?=E2=9C=A8=20Add=20typed=20OPA=20config=20sc?= =?UTF-8?q?hema?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add shared typed OPA config structs, wire the new System and ProjectConfig fields through the config merge path, and validate unknown top-level keys during config loading and reconciliation. Closes #616 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- api/config/v2alpha2/opaconfig_types.go | 77 +++++ api/config/v2alpha2/projectconfig_types.go | 5 + api/config/v2alpha2/zz_generated.deepcopy.go | 173 +++++++++++ api/styra/v1beta1/opaconfig_types.go | 77 +++++ api/styra/v1beta1/system_types.go | 28 +- api/styra/v1beta1/zz_generated.deepcopy.go | 195 +++++++++++- .../crd/bases/styra.bankdata.dk_systems.yaml | 170 +++++++++- internal/config/config_test.go | 23 +- .../controller/styra/system_controller.go | 57 +++- internal/k8sconv/k8sconv.go | 6 +- internal/k8sconv/k8sconv_test.go | 293 ++++++++++++++++++ pkg/opaconfig/opaconfig.go | 97 ++++++ pkg/opaconfig/opaconfig_test.go | 64 ++++ 13 files changed, 1234 insertions(+), 31 deletions(-) create mode 100644 api/config/v2alpha2/opaconfig_types.go create mode 100644 api/styra/v1beta1/opaconfig_types.go create mode 100644 pkg/opaconfig/opaconfig.go create mode 100644 pkg/opaconfig/opaconfig_test.go diff --git a/api/config/v2alpha2/opaconfig_types.go b/api/config/v2alpha2/opaconfig_types.go new file mode 100644 index 00000000..bf0545c1 --- /dev/null +++ b/api/config/v2alpha2/opaconfig_types.go @@ -0,0 +1,77 @@ +/* +Copyright (C) 2025 Bankdata (bankdata@bankdata.dk) + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v2alpha2 + +import ( + "encoding/json" + + "k8s.io/apimachinery/pkg/runtime" + + "github.com/bankdata/styra-controller/pkg/opaconfig" +) + +// OPAConfigSpec mirrors the official OPA configuration schema. +type OPAConfigSpec struct { + Services *runtime.RawExtension `json:"services,omitempty"` + Labels map[string]string `json:"labels,omitempty"` + Discovery *runtime.RawExtension `json:"discovery,omitempty"` + Bundle *runtime.RawExtension `json:"bundle,omitempty"` + Bundles *runtime.RawExtension `json:"bundles,omitempty"` + DecisionLogs *runtime.RawExtension `json:"decision_logs,omitempty"` + Status *runtime.RawExtension `json:"status,omitempty"` + Plugins map[string]*runtime.RawExtension `json:"plugins,omitempty"` + Keys *runtime.RawExtension `json:"keys,omitempty"` + DefaultDecision *string `json:"default_decision,omitempty"` + DefaultAuthorizationDecision *string `json:"default_authorization_decision,omitempty"` + Caching *runtime.RawExtension `json:"caching,omitempty"` + NDBuiltinCache bool `json:"nd_builtin_cache,omitempty"` + PersistenceDirectory *string `json:"persistence_directory,omitempty"` + DistributedTracing *runtime.RawExtension `json:"distributed_tracing,omitempty"` + MetricsExport *runtime.RawExtension `json:"metrics_export,omitempty"` + Server *OPAServerConfig `json:"server,omitempty"` + Storage *OPAStorageConfig `json:"storage,omitempty"` +} + +// OPAServerConfig mirrors OPA's server configuration section. +type OPAServerConfig struct { + Metrics *runtime.RawExtension `json:"metrics,omitempty"` + Encoding *runtime.RawExtension `json:"encoding,omitempty"` + Decoding *runtime.RawExtension `json:"decoding,omitempty"` + LoggerPlugin *string `json:"logger_plugin,omitempty"` +} + +// OPAStorageConfig mirrors OPA's storage configuration section. +type OPAStorageConfig struct { + Disk *runtime.RawExtension `json:"disk,omitempty"` +} + +// UnmarshalJSON validates the OPA config against the known schema keys before +// decoding it. +func (c *OPAConfigSpec) UnmarshalJSON(data []byte) error { + if err := opaconfig.ValidateRaw(data); err != nil { + return err + } + + type alias OPAConfigSpec + var decoded alias + if err := json.Unmarshal(data, &decoded); err != nil { + return err + } + + *c = OPAConfigSpec(decoded) + return nil +} diff --git a/api/config/v2alpha2/projectconfig_types.go b/api/config/v2alpha2/projectconfig_types.go index 5c1f25a6..ca91f409 100644 --- a/api/config/v2alpha2/projectconfig_types.go +++ b/api/config/v2alpha2/projectconfig_types.go @@ -54,8 +54,13 @@ type ProjectConfig struct { LeaderElection *LeaderElectionConfig `json:"leaderElection"` + // Deprecated: OPA contains the legacy controller-managed OPA settings. OPA OPAConfig `json:"opa,omitempty"` + // OPAConfig contains the full OPA configuration schema and takes + // precedence over the legacy OPA settings. + OPAConfig *OPAConfigSpec `json:"opaConfig,omitempty"` + // SystemPrefix is a prefix for all the systems that the controller creates. SystemPrefix string `json:"systemPrefix"` diff --git a/api/config/v2alpha2/zz_generated.deepcopy.go b/api/config/v2alpha2/zz_generated.deepcopy.go index 32032c92..ab8dc370 100644 --- a/api/config/v2alpha2/zz_generated.deepcopy.go +++ b/api/config/v2alpha2/zz_generated.deepcopy.go @@ -242,6 +242,119 @@ func (in *OPAConfig) DeepCopy() *OPAConfig { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OPAConfigSpec) DeepCopyInto(out *OPAConfigSpec) { + *out = *in + if in.Services != nil { + in, out := &in.Services, &out.Services + *out = new(runtime.RawExtension) + (*in).DeepCopyInto(*out) + } + if in.Labels != nil { + in, out := &in.Labels, &out.Labels + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } + if in.Discovery != nil { + in, out := &in.Discovery, &out.Discovery + *out = new(runtime.RawExtension) + (*in).DeepCopyInto(*out) + } + if in.Bundle != nil { + in, out := &in.Bundle, &out.Bundle + *out = new(runtime.RawExtension) + (*in).DeepCopyInto(*out) + } + if in.Bundles != nil { + in, out := &in.Bundles, &out.Bundles + *out = new(runtime.RawExtension) + (*in).DeepCopyInto(*out) + } + if in.DecisionLogs != nil { + in, out := &in.DecisionLogs, &out.DecisionLogs + *out = new(runtime.RawExtension) + (*in).DeepCopyInto(*out) + } + if in.Status != nil { + in, out := &in.Status, &out.Status + *out = new(runtime.RawExtension) + (*in).DeepCopyInto(*out) + } + if in.Plugins != nil { + in, out := &in.Plugins, &out.Plugins + *out = make(map[string]*runtime.RawExtension, len(*in)) + for key, val := range *in { + var outVal *runtime.RawExtension + if val == nil { + (*out)[key] = nil + } else { + inVal := (*in)[key] + in, out := &inVal, &outVal + *out = new(runtime.RawExtension) + (*in).DeepCopyInto(*out) + } + (*out)[key] = outVal + } + } + if in.Keys != nil { + in, out := &in.Keys, &out.Keys + *out = new(runtime.RawExtension) + (*in).DeepCopyInto(*out) + } + if in.DefaultDecision != nil { + in, out := &in.DefaultDecision, &out.DefaultDecision + *out = new(string) + **out = **in + } + if in.DefaultAuthorizationDecision != nil { + in, out := &in.DefaultAuthorizationDecision, &out.DefaultAuthorizationDecision + *out = new(string) + **out = **in + } + if in.Caching != nil { + in, out := &in.Caching, &out.Caching + *out = new(runtime.RawExtension) + (*in).DeepCopyInto(*out) + } + if in.PersistenceDirectory != nil { + in, out := &in.PersistenceDirectory, &out.PersistenceDirectory + *out = new(string) + **out = **in + } + if in.DistributedTracing != nil { + in, out := &in.DistributedTracing, &out.DistributedTracing + *out = new(runtime.RawExtension) + (*in).DeepCopyInto(*out) + } + if in.MetricsExport != nil { + in, out := &in.MetricsExport, &out.MetricsExport + *out = new(runtime.RawExtension) + (*in).DeepCopyInto(*out) + } + if in.Server != nil { + in, out := &in.Server, &out.Server + *out = new(OPAServerConfig) + (*in).DeepCopyInto(*out) + } + if in.Storage != nil { + in, out := &in.Storage, &out.Storage + *out = new(OPAStorageConfig) + (*in).DeepCopyInto(*out) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OPAConfigSpec. +func (in *OPAConfigSpec) DeepCopy() *OPAConfigSpec { + if in == nil { + return nil + } + out := new(OPAConfigSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *OPAControlPlaneConfig) DeepCopyInto(out *OPAControlPlaneConfig) { *out = *in @@ -278,6 +391,61 @@ func (in *OPAControlPlaneConfig) DeepCopy() *OPAControlPlaneConfig { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OPAServerConfig) DeepCopyInto(out *OPAServerConfig) { + *out = *in + if in.Metrics != nil { + in, out := &in.Metrics, &out.Metrics + *out = new(runtime.RawExtension) + (*in).DeepCopyInto(*out) + } + if in.Encoding != nil { + in, out := &in.Encoding, &out.Encoding + *out = new(runtime.RawExtension) + (*in).DeepCopyInto(*out) + } + if in.Decoding != nil { + in, out := &in.Decoding, &out.Decoding + *out = new(runtime.RawExtension) + (*in).DeepCopyInto(*out) + } + if in.LoggerPlugin != nil { + in, out := &in.LoggerPlugin, &out.LoggerPlugin + *out = new(string) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OPAServerConfig. +func (in *OPAServerConfig) DeepCopy() *OPAServerConfig { + if in == nil { + return nil + } + out := new(OPAServerConfig) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OPAStorageConfig) DeepCopyInto(out *OPAStorageConfig) { + *out = *in + if in.Disk != nil { + in, out := &in.Disk, &out.Disk + *out = new(runtime.RawExtension) + (*in).DeepCopyInto(*out) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OPAStorageConfig. +func (in *OPAStorageConfig) DeepCopy() *OPAStorageConfig { + if in == nil { + return nil + } + out := new(OPAStorageConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ProjectConfig) DeepCopyInto(out *ProjectConfig) { *out = *in @@ -293,6 +461,11 @@ func (in *ProjectConfig) DeepCopyInto(out *ProjectConfig) { **out = **in } in.OPA.DeepCopyInto(&out.OPA) + if in.OPAConfig != nil { + in, out := &in.OPAConfig, &out.OPAConfig + *out = new(OPAConfigSpec) + (*in).DeepCopyInto(*out) + } if in.OPAControlPlaneConfig != nil { in, out := &in.OPAControlPlaneConfig, &out.OPAControlPlaneConfig *out = new(OPAControlPlaneConfig) diff --git a/api/styra/v1beta1/opaconfig_types.go b/api/styra/v1beta1/opaconfig_types.go new file mode 100644 index 00000000..929bb6e5 --- /dev/null +++ b/api/styra/v1beta1/opaconfig_types.go @@ -0,0 +1,77 @@ +/* +Copyright (C) 2025 Bankdata (bankdata@bankdata.dk) + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1beta1 + +import ( + "encoding/json" + + "k8s.io/apimachinery/pkg/runtime" + + "github.com/bankdata/styra-controller/pkg/opaconfig" +) + +// OPAConfigSpec mirrors the official OPA configuration schema. +type OPAConfigSpec struct { + Services *runtime.RawExtension `json:"services,omitempty"` + Labels map[string]string `json:"labels,omitempty"` + Discovery *runtime.RawExtension `json:"discovery,omitempty"` + Bundle *runtime.RawExtension `json:"bundle,omitempty"` + Bundles *runtime.RawExtension `json:"bundles,omitempty"` + DecisionLogs *runtime.RawExtension `json:"decision_logs,omitempty"` + Status *runtime.RawExtension `json:"status,omitempty"` + Plugins map[string]*runtime.RawExtension `json:"plugins,omitempty"` + Keys *runtime.RawExtension `json:"keys,omitempty"` + DefaultDecision *string `json:"default_decision,omitempty"` + DefaultAuthorizationDecision *string `json:"default_authorization_decision,omitempty"` + Caching *runtime.RawExtension `json:"caching,omitempty"` + NDBuiltinCache bool `json:"nd_builtin_cache,omitempty"` + PersistenceDirectory *string `json:"persistence_directory,omitempty"` + DistributedTracing *runtime.RawExtension `json:"distributed_tracing,omitempty"` + MetricsExport *runtime.RawExtension `json:"metrics_export,omitempty"` + Server *OPAServerConfig `json:"server,omitempty"` + Storage *OPAStorageConfig `json:"storage,omitempty"` +} + +// OPAServerConfig mirrors OPA's server configuration section. +type OPAServerConfig struct { + Metrics *runtime.RawExtension `json:"metrics,omitempty"` + Encoding *runtime.RawExtension `json:"encoding,omitempty"` + Decoding *runtime.RawExtension `json:"decoding,omitempty"` + LoggerPlugin *string `json:"logger_plugin,omitempty"` +} + +// OPAStorageConfig mirrors OPA's storage configuration section. +type OPAStorageConfig struct { + Disk *runtime.RawExtension `json:"disk,omitempty"` +} + +// UnmarshalJSON validates the OPA config against the known schema keys before +// decoding it. +func (c *OPAConfigSpec) UnmarshalJSON(data []byte) error { + if err := opaconfig.ValidateRaw(data); err != nil { + return err + } + + type alias OPAConfigSpec + var decoded alias + if err := json.Unmarshal(data, &decoded); err != nil { + return err + } + + *c = OPAConfigSpec(decoded) + return nil +} diff --git a/api/styra/v1beta1/system_types.go b/api/styra/v1beta1/system_types.go index 36587e6a..71602814 100644 --- a/api/styra/v1beta1/system_types.go +++ b/api/styra/v1beta1/system_types.go @@ -43,17 +43,33 @@ type SystemSpec struct { // Datasources represents a list of datasources to be mounted in the system. Datasources []Datasource `json:"datasources,omitempty"` - // DiscoveryOverrides is an OPA config which will take precedence over the - // configuration supplied by the OPA discovery API. Configuration set here - // will be merged with the configuration supplied by the discovery API. + // Deprecated: DiscoveryOverrides is unused by the controller and will be + // removed in a future version. Use OPA.Config instead. DiscoveryOverrides *DiscoveryOverrides `json:"discoveryOverrides,omitempty"` SourceControl *SourceControl `json:"sourceControl,omitempty"` LocalPlane *LocalPlane `json:"localPlane,omitempty"` - // CustomOPAConfig allows the owner of a System resource to set custom features - // without having to extend the Controller - CustomOPAConfig *runtime.RawExtension `json:"customOPAConfig,omitempty"` + // Deprecated: CustomOPAConfig allows the owner of a System resource to set + // custom OPA configuration that is merged into the generated OPA config. + // Use OPA.Config instead, which provides the same capability with runtime + // validation of field names against the OPA configuration schema. + // If both are set, OPA.Config takes precedence on conflicting keys. + CustomOPAConfig *OPAConfigSpec `json:"customOPAConfig,omitempty"` + + // OPA contains OPA-specific configuration for this system. + OPA *SystemOPASpec `json:"opa,omitempty"` +} + +// SystemOPASpec contains OPA-specific configuration for a System. +type SystemOPASpec struct { + // Config accepts any valid OPA configuration YAML. The top-level keys are + // validated by the OPA configuration schema (see + // https://github.com/open-policy-agent/opa/blob/main/v1/config/config.go). + // + // Configuration set here is merged on top of the generated OPA config and + // takes precedence over the deprecated customOPAConfig field on conflicts. + Config *OPAConfigSpec `json:"config,omitempty"` } // DiscoveryOverrides specifies system specific overrides for the configuration diff --git a/api/styra/v1beta1/zz_generated.deepcopy.go b/api/styra/v1beta1/zz_generated.deepcopy.go index bdcf0a30..55cb210d 100644 --- a/api/styra/v1beta1/zz_generated.deepcopy.go +++ b/api/styra/v1beta1/zz_generated.deepcopy.go @@ -217,6 +217,119 @@ func (in *OPAConfigDistributedTracing) DeepCopy() *OPAConfigDistributedTracing { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OPAConfigSpec) DeepCopyInto(out *OPAConfigSpec) { + *out = *in + if in.Services != nil { + in, out := &in.Services, &out.Services + *out = new(runtime.RawExtension) + (*in).DeepCopyInto(*out) + } + if in.Labels != nil { + in, out := &in.Labels, &out.Labels + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } + if in.Discovery != nil { + in, out := &in.Discovery, &out.Discovery + *out = new(runtime.RawExtension) + (*in).DeepCopyInto(*out) + } + if in.Bundle != nil { + in, out := &in.Bundle, &out.Bundle + *out = new(runtime.RawExtension) + (*in).DeepCopyInto(*out) + } + if in.Bundles != nil { + in, out := &in.Bundles, &out.Bundles + *out = new(runtime.RawExtension) + (*in).DeepCopyInto(*out) + } + if in.DecisionLogs != nil { + in, out := &in.DecisionLogs, &out.DecisionLogs + *out = new(runtime.RawExtension) + (*in).DeepCopyInto(*out) + } + if in.Status != nil { + in, out := &in.Status, &out.Status + *out = new(runtime.RawExtension) + (*in).DeepCopyInto(*out) + } + if in.Plugins != nil { + in, out := &in.Plugins, &out.Plugins + *out = make(map[string]*runtime.RawExtension, len(*in)) + for key, val := range *in { + var outVal *runtime.RawExtension + if val == nil { + (*out)[key] = nil + } else { + inVal := (*in)[key] + in, out := &inVal, &outVal + *out = new(runtime.RawExtension) + (*in).DeepCopyInto(*out) + } + (*out)[key] = outVal + } + } + if in.Keys != nil { + in, out := &in.Keys, &out.Keys + *out = new(runtime.RawExtension) + (*in).DeepCopyInto(*out) + } + if in.DefaultDecision != nil { + in, out := &in.DefaultDecision, &out.DefaultDecision + *out = new(string) + **out = **in + } + if in.DefaultAuthorizationDecision != nil { + in, out := &in.DefaultAuthorizationDecision, &out.DefaultAuthorizationDecision + *out = new(string) + **out = **in + } + if in.Caching != nil { + in, out := &in.Caching, &out.Caching + *out = new(runtime.RawExtension) + (*in).DeepCopyInto(*out) + } + if in.PersistenceDirectory != nil { + in, out := &in.PersistenceDirectory, &out.PersistenceDirectory + *out = new(string) + **out = **in + } + if in.DistributedTracing != nil { + in, out := &in.DistributedTracing, &out.DistributedTracing + *out = new(runtime.RawExtension) + (*in).DeepCopyInto(*out) + } + if in.MetricsExport != nil { + in, out := &in.MetricsExport, &out.MetricsExport + *out = new(runtime.RawExtension) + (*in).DeepCopyInto(*out) + } + if in.Server != nil { + in, out := &in.Server, &out.Server + *out = new(OPAServerConfig) + (*in).DeepCopyInto(*out) + } + if in.Storage != nil { + in, out := &in.Storage, &out.Storage + *out = new(OPAStorageConfig) + (*in).DeepCopyInto(*out) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OPAConfigSpec. +func (in *OPAConfigSpec) DeepCopy() *OPAConfigSpec { + if in == nil { + return nil + } + out := new(OPAConfigSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *OPAConfigStatus) DeepCopyInto(out *OPAConfigStatus) { *out = *in @@ -232,6 +345,61 @@ func (in *OPAConfigStatus) DeepCopy() *OPAConfigStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OPAServerConfig) DeepCopyInto(out *OPAServerConfig) { + *out = *in + if in.Metrics != nil { + in, out := &in.Metrics, &out.Metrics + *out = new(runtime.RawExtension) + (*in).DeepCopyInto(*out) + } + if in.Encoding != nil { + in, out := &in.Encoding, &out.Encoding + *out = new(runtime.RawExtension) + (*in).DeepCopyInto(*out) + } + if in.Decoding != nil { + in, out := &in.Decoding, &out.Decoding + *out = new(runtime.RawExtension) + (*in).DeepCopyInto(*out) + } + if in.LoggerPlugin != nil { + in, out := &in.LoggerPlugin, &out.LoggerPlugin + *out = new(string) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OPAServerConfig. +func (in *OPAServerConfig) DeepCopy() *OPAServerConfig { + if in == nil { + return nil + } + out := new(OPAServerConfig) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OPAStorageConfig) DeepCopyInto(out *OPAStorageConfig) { + *out = *in + if in.Disk != nil { + in, out := &in.Disk, &out.Disk + *out = new(runtime.RawExtension) + (*in).DeepCopyInto(*out) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OPAStorageConfig. +func (in *OPAStorageConfig) DeepCopy() *OPAStorageConfig { + if in == nil { + return nil + } + out := new(OPAStorageConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ReasonMapping) DeepCopyInto(out *ReasonMapping) { *out = *in @@ -337,6 +505,26 @@ func (in *SystemList) DeepCopyObject() runtime.Object { return nil } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *SystemOPASpec) DeepCopyInto(out *SystemOPASpec) { + *out = *in + if in.Config != nil { + in, out := &in.Config, &out.Config + *out = new(OPAConfigSpec) + (*in).DeepCopyInto(*out) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SystemOPASpec. +func (in *SystemOPASpec) DeepCopy() *SystemOPASpec { + if in == nil { + return nil + } + out := new(SystemOPASpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SystemSpec) DeepCopyInto(out *SystemSpec) { *out = *in @@ -384,7 +572,12 @@ func (in *SystemSpec) DeepCopyInto(out *SystemSpec) { } if in.CustomOPAConfig != nil { in, out := &in.CustomOPAConfig, &out.CustomOPAConfig - *out = new(runtime.RawExtension) + *out = new(OPAConfigSpec) + (*in).DeepCopyInto(*out) + } + if in.OPA != nil { + in, out := &in.OPA, &out.OPA + *out = new(SystemOPASpec) (*in).DeepCopyInto(*out) } } diff --git a/config/crd/bases/styra.bankdata.dk_systems.yaml b/config/crd/bases/styra.bankdata.dk_systems.yaml index 2a4dc23e..40063c98 100644 --- a/config/crd/bases/styra.bankdata.dk_systems.yaml +++ b/config/crd/bases/styra.bankdata.dk_systems.yaml @@ -51,10 +51,84 @@ spec: properties: customOPAConfig: description: |- - CustomOPAConfig allows the owner of a System resource to set custom features - without having to extend the Controller + Deprecated: CustomOPAConfig allows the owner of a System resource to set + custom OPA configuration that is merged into the generated OPA config. + Use OPA.Config instead, which provides the same capability with runtime + validation of field names against the OPA configuration schema. + If both are set, OPA.Config takes precedence on conflicting keys. + properties: + bundle: + type: object + x-kubernetes-preserve-unknown-fields: true + bundles: + type: object + x-kubernetes-preserve-unknown-fields: true + caching: + type: object + x-kubernetes-preserve-unknown-fields: true + decision_logs: + type: object + x-kubernetes-preserve-unknown-fields: true + default_authorization_decision: + type: string + default_decision: + type: string + discovery: + type: object + x-kubernetes-preserve-unknown-fields: true + distributed_tracing: + type: object + x-kubernetes-preserve-unknown-fields: true + keys: + type: object + x-kubernetes-preserve-unknown-fields: true + labels: + additionalProperties: + type: string + type: object + metrics_export: + type: object + x-kubernetes-preserve-unknown-fields: true + nd_builtin_cache: + type: boolean + persistence_directory: + type: string + plugins: + additionalProperties: + type: object + x-kubernetes-preserve-unknown-fields: true + type: object + server: + description: OPAServerConfig mirrors OPA's server configuration + section. + properties: + decoding: + type: object + x-kubernetes-preserve-unknown-fields: true + encoding: + type: object + x-kubernetes-preserve-unknown-fields: true + logger_plugin: + type: string + metrics: + type: object + x-kubernetes-preserve-unknown-fields: true + type: object + services: + type: object + x-kubernetes-preserve-unknown-fields: true + status: + type: object + x-kubernetes-preserve-unknown-fields: true + storage: + description: OPAStorageConfig mirrors OPA's storage configuration + section. + properties: + disk: + type: object + x-kubernetes-preserve-unknown-fields: true + type: object type: object - x-kubernetes-preserve-unknown-fields: true datasources: description: Datasources represents a list of datasources to be mounted in the system. @@ -159,9 +233,8 @@ spec: type: boolean discoveryOverrides: description: |- - DiscoveryOverrides is an OPA config which will take precedence over the - configuration supplied by the OPA discovery API. Configuration set here - will be merged with the configuration supplied by the discovery API. + Deprecated: DiscoveryOverrides is unused by the controller and will be + removed in a future version. Use OPA.Config instead. properties: distributed_tracing: description: |- @@ -215,6 +288,91 @@ spec: required: - name type: object + opa: + description: OPA contains OPA-specific configuration for this system. + properties: + config: + description: |- + Config accepts any valid OPA configuration YAML. The top-level keys are + validated by the OPA configuration schema (see + https://github.com/open-policy-agent/opa/blob/main/v1/config/config.go). + + Configuration set here is merged on top of the generated OPA config and + takes precedence over the deprecated customOPAConfig field on conflicts. + properties: + bundle: + type: object + x-kubernetes-preserve-unknown-fields: true + bundles: + type: object + x-kubernetes-preserve-unknown-fields: true + caching: + type: object + x-kubernetes-preserve-unknown-fields: true + decision_logs: + type: object + x-kubernetes-preserve-unknown-fields: true + default_authorization_decision: + type: string + default_decision: + type: string + discovery: + type: object + x-kubernetes-preserve-unknown-fields: true + distributed_tracing: + type: object + x-kubernetes-preserve-unknown-fields: true + keys: + type: object + x-kubernetes-preserve-unknown-fields: true + labels: + additionalProperties: + type: string + type: object + metrics_export: + type: object + x-kubernetes-preserve-unknown-fields: true + nd_builtin_cache: + type: boolean + persistence_directory: + type: string + plugins: + additionalProperties: + type: object + x-kubernetes-preserve-unknown-fields: true + type: object + server: + description: OPAServerConfig mirrors OPA's server configuration + section. + properties: + decoding: + type: object + x-kubernetes-preserve-unknown-fields: true + encoding: + type: object + x-kubernetes-preserve-unknown-fields: true + logger_plugin: + type: string + metrics: + type: object + x-kubernetes-preserve-unknown-fields: true + type: object + services: + type: object + x-kubernetes-preserve-unknown-fields: true + status: + type: object + x-kubernetes-preserve-unknown-fields: true + storage: + description: OPAStorageConfig mirrors OPA's storage configuration + section. + properties: + disk: + type: object + x-kubernetes-preserve-unknown-fields: true + type: object + type: object + type: object sourceControl: description: SourceControl holds SourceControl configuration. properties: diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 6c4020f3..59cb2074 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -196,6 +196,19 @@ not: valid: yaml: {{ gomega.Ω(err).Should(gomega.HaveOccurred()) }) + ginkgo.It("returns an error when opaConfig contains an unknown key", func() { + f := writeFile("opa-config.yaml", ` +apiVersion: config.bankdata.dk/v2alpha2 +kind: ProjectConfig +opaConfig: + servicess: + - name: bundle + url: https://example.invalid +`) + _, err := Load([]string{f}, scheme) + gomega.Ω(err).Should(gomega.HaveOccurred()) + }) + ginkgo.It("preserves nested OPA config when overlay only sets secrets", func() { base := writeFile("base.yaml", ` apiVersion: config.bankdata.dk/v2alpha2 @@ -221,10 +234,10 @@ opaControlPlaneConfig: `) cfg, err := Load([]string{base, secrets}, scheme) gomega.Ω(err).ShouldNot(gomega.HaveOccurred()) - gomega.Ω(cfg.OPA.BundleServer).ShouldNot(gomega.BeNil()) - gomega.Ω(cfg.OPA.BundleServer.URL).Should(gomega.Equal("https://minio-host")) - gomega.Ω(cfg.OPA.BundleServer.Path).Should(gomega.Equal("/ocp")) - gomega.Ω(cfg.OPA.Metrics.Prometheus.HTTP.Buckets).Should(gomega.Equal([]float64{0.01, 0.1, 1})) + gomega.Ω(cfg.OPA.BundleServer).ShouldNot(gomega.BeNil()) //nolint:staticcheck + gomega.Ω(cfg.OPA.BundleServer.URL).Should(gomega.Equal("https://minio-host")) //nolint:staticcheck + gomega.Ω(cfg.OPA.BundleServer.Path).Should(gomega.Equal("/ocp")) //nolint:staticcheck + gomega.Ω(cfg.OPA.Metrics.Prometheus.HTTP.Buckets).Should(gomega.Equal([]float64{0.01, 0.1, 1})) //nolint:staticcheck gomega.Ω(cfg.OPAControlPlaneConfig.Token).Should(gomega.Equal("secret-token")) }) @@ -340,7 +353,7 @@ opaControlPlaneConfig: gomega.Ω(cfg.OPAControlPlaneConfig.Address).Should(gomega.Equal("https://ocp.example.com")) gomega.Ω(cfg.OPAControlPlaneConfig.BundleObjectStorage.S3.Bucket).Should(gomega.Equal("bundles")) gomega.Ω(cfg.OPAControlPlaneConfig.DefaultRequirements).Should(gomega.Equal([]string{"base-library"})) - gomega.Ω(cfg.OPA.BundleServer.URL).Should(gomega.Equal("https://s3.example.com")) + gomega.Ω(cfg.OPA.BundleServer.URL).Should(gomega.Equal("https://s3.example.com")) //nolint:staticcheck // Secret fields from Secret overlay gomega.Ω(cfg.OPAControlPlaneConfig.Token).Should(gomega.Equal("real-ocp-token")) diff --git a/internal/controller/styra/system_controller.go b/internal/controller/styra/system_controller.go index 11108e22..1e348c19 100644 --- a/internal/controller/styra/system_controller.go +++ b/internal/controller/styra/system_controller.go @@ -28,7 +28,6 @@ import ( "github.com/go-logr/logr" "github.com/pkg/errors" "github.com/prometheus/client_golang/prometheus" - "gopkg.in/yaml.v2" corev1 "k8s.io/api/core/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -55,6 +54,7 @@ import ( "github.com/bankdata/styra-controller/internal/webhook" "github.com/bankdata/styra-controller/pkg/httperror" "github.com/bankdata/styra-controller/pkg/ocp" + opaconfigutil "github.com/bankdata/styra-controller/pkg/opaconfig" ) const ( @@ -424,15 +424,41 @@ func (r *SystemReconciler) reconcileOPAConfigMapForOCP( log.Info("Reconciling OPA ConfigMap") var expectedOPAConfigMap corev1.ConfigMap + var projectConfig map[string]interface{} + if r.Config.OPAConfig != nil { + var err error + projectConfig, err = opaconfigutil.ToMap(r.Config.OPAConfig) + if err != nil { + return ctrl.Result{}, false, ctrlerr.Wrap(err, "Could not convert controller OPA config"). + WithEvent(v1beta1.EventErrorConvertOPAConf). + WithSystemCondition(v1beta1.ConditionTypeOPAConfigMapUpdated) + } + } + var customConfig map[string]interface{} - if system.Spec.CustomOPAConfig != nil { - err := yaml.Unmarshal(system.Spec.CustomOPAConfig.Raw, &customConfig) + if system.Spec.CustomOPAConfig != nil { //nolint:staticcheck + var err error + customConfig, err = opaconfigutil.ToMap(system.Spec.CustomOPAConfig) //nolint:staticcheck if err != nil { return ctrl.Result{}, false, err } } - bundleURL, err := url.JoinPath(r.Config.OPA.BundleServer.URL, r.Config.OPA.BundleServer.Path) + var opaConfig map[string]interface{} + if system.Spec.OPA != nil && system.Spec.OPA.Config != nil { + var err error + opaConfig, err = opaconfigutil.ToMap(system.Spec.OPA.Config) + if err != nil { + return ctrl.Result{}, false, ctrlerr.Wrap(err, "Could not convert spec.opa.config"). + WithEvent(v1beta1.EventErrorConvertOPAConf). + WithSystemCondition(v1beta1.ConditionTypeOPAConfigMapUpdated) + } + } + + legacyOPA := r.Config.OPA //nolint:staticcheck + legacyBundleServer := legacyOPA.BundleServer + legacyDecisionAPIConfig := legacyOPA.DecisionAPIConfig + bundleURL, err := url.JoinPath(legacyBundleServer.URL, legacyBundleServer.Path) if err != nil { return ctrl.Result{}, false, ctrlerr.Wrap(err, "Invalid OPA BundleServer URL or path"). WithEvent(v1beta1.EventErrorConvertOPAConf). @@ -444,36 +470,43 @@ func (r *SystemReconciler) reconcileOPAConfigMapForOCP( S3EnvironmentCredentials: map[string]ocp.EmptyStruct{}, }, } - if r.Config.OPA.BundleServer.TokenPath != "" { + if legacyBundleServer.TokenPath != "" { bundleServiceCredentials = &ocp.ServiceCredentials{ Bearer: &ocp.Bearer{ - TokenPath: r.Config.OPA.BundleServer.TokenPath, + TokenPath: legacyBundleServer.TokenPath, }, } } opaconf := ocp.OPAConfig{ BundleService: &ocp.OPAServiceConfig{ - Name: r.Config.OPA.BundleServer.Name, + Name: legacyBundleServer.Name, URL: bundleURL, Credentials: bundleServiceCredentials, }, LogService: &ocp.OPAServiceConfig{ - Name: r.Config.OPA.DecisionAPIConfig.Name, - URL: r.Config.OPA.DecisionAPIConfig.ServiceURL, + Name: legacyDecisionAPIConfig.Name, + URL: legacyDecisionAPIConfig.ServiceURL, Credentials: &ocp.ServiceCredentials{ Bearer: &ocp.Bearer{ - TokenPath: r.Config.OPA.DecisionAPIConfig.TokenPath, + TokenPath: legacyDecisionAPIConfig.TokenPath, }, }, }, - DecisionLogReporting: r.Config.OPA.DecisionAPIConfig.Reporting, + DecisionLogReporting: legacyDecisionAPIConfig.Reporting, BundleResource: fmt.Sprintf("bundles/%s/bundle.tar.gz", uniqueName), UniqueName: uniqueName, Namespace: system.Namespace, } - expectedOPAConfigMap, err = k8sconv.OPAConfToK8sOPAConfigMapforOCP(opaconf, r.Config.OPA, customConfig, log) + expectedOPAConfigMap, err = k8sconv.OPAConfToK8sOPAConfigMapforOCP( + opaconf, + legacyOPA, + projectConfig, + customConfig, + opaConfig, + log, + ) if err != nil { return ctrl.Result{}, false, ctrlerr.Wrap(err, "Could not convert OPA conf to ConfigMap"). WithEvent(v1beta1.EventErrorConvertOPAConf). diff --git a/internal/k8sconv/k8sconv.go b/internal/k8sconv/k8sconv.go index 9ac16758..45c16f79 100644 --- a/internal/k8sconv/k8sconv.go +++ b/internal/k8sconv/k8sconv.go @@ -141,7 +141,9 @@ type HTTPMetricsConfig struct { func OPAConfToK8sOPAConfigMapforOCP( opaconf ocp.OPAConfig, opaDefaultConfig configv2alpha2.OPAConfig, + projectConfig map[string]interface{}, customConfig map[string]interface{}, + opaConfig map[string]interface{}, _ logr.Logger, ) (corev1.ConfigMap, error) { var services []*ocp.OPAServiceConfig @@ -209,7 +211,9 @@ func OPAConfToK8sOPAConfigMapforOCP( return corev1.ConfigMap{}, err } - merged := mergeMaps(opaConfigMapMapStringInterface, customConfig) + merged := mergeMaps(opaConfigMapMapStringInterface, projectConfig) + merged = mergeMaps(merged, customConfig) + merged = mergeMaps(merged, opaConfig) res, err := yaml.Marshal(&merged) if err != nil { diff --git a/internal/k8sconv/k8sconv_test.go b/internal/k8sconv/k8sconv_test.go index 9b872923..d0f58c15 100644 --- a/internal/k8sconv/k8sconv_test.go +++ b/internal/k8sconv/k8sconv_test.go @@ -43,7 +43,9 @@ var _ = ginkgo.Describe("OPAConfToK8sOPAConfigMap", func() { cm, err := k8sconv.OPAConfToK8sOPAConfigMapforOCP( test.opaconf, test.opaDefaultConfig, + nil, test.customConfig, + nil, logr.Discard()) gomega.Expect(err).To(gomega.BeNil()) @@ -73,6 +75,8 @@ var _ = ginkgo.Describe("OPAConfToK8sOPAConfigMap", func() { PersistBundleDirectory: "/opa-bundles", }, opaconf: ocp.OPAConfig{ + UniqueName: "system", + Namespace: "system-ns", BundleResource: "bundles/system/bundle.tar.gz", BundleService: &ocp.OPAServiceConfig{ Name: "s3", @@ -127,6 +131,9 @@ bundles: persist: true test: 123 persistence_directory: /opa-bundles +labels: + namespace: system-ns + unique-name: system decision_logs: reporting: upload_size_limit_bytes: 1 @@ -162,7 +169,9 @@ var _ = ginkgo.Describe("OPAConfToK8sOPAConfigMap", func() { cm, err := k8sconv.OPAConfToK8sOPAConfigMapforOCP( test.opaconf, test.opaDefaultConfig, + nil, test.customConfig, + nil, logr.Discard()) gomega.Expect(err).To(gomega.BeNil()) @@ -262,3 +271,287 @@ distributed_tracing: }), ) }) + +// Test that spec.opa.config (opaConfig) takes highest precedence over customConfig on conflict. +var _ = ginkgo.Describe("OPAConfToK8sOPAConfigMap opaConfig precedence", func() { + + type test struct { + opaDefaultConfig configv2alpha2.OPAConfig + opaconf ocp.OPAConfig + customConfig map[string]interface{} + opaConfig map[string]interface{} + expectedCMContent string + } + + ginkgo.DescribeTable("OPAConfToK8sOPAConfigMap", func(test test) { + cm, err := k8sconv.OPAConfToK8sOPAConfigMapforOCP( + test.opaconf, + test.opaDefaultConfig, + nil, + test.customConfig, + test.opaConfig, + logr.Discard()) + + gomega.Expect(err).To(gomega.BeNil()) + + var actualMap, expectedMap map[string]interface{} + actualYAML := cm.Data["opa-conf.yaml"] + expectedYAML := test.expectedCMContent + + err = yaml.Unmarshal([]byte(actualYAML), &actualMap) + gomega.Expect(err).ToNot(gomega.HaveOccurred(), "Failed to unmarshal actual YAML") + + err = yaml.Unmarshal([]byte(expectedYAML), &expectedMap) + gomega.Expect(err).ToNot(gomega.HaveOccurred(), "Failed to unmarshal expected YAML") + + gomega.Expect(actualMap).To(gomega.Equal(expectedMap)) + }, + ginkgo.Entry("opaConfig wins over customConfig on conflict", test{ + opaconf: ocp.OPAConfig{ + BundleResource: "bundles/system/bundle.tar.gz", + BundleService: &ocp.OPAServiceConfig{ + Name: "s3", + URL: "https://minio/ocp", + Credentials: &ocp.ServiceCredentials{ + S3: &ocp.S3Signing{ + S3EnvironmentCredentials: map[string]ocp.EmptyStruct{}, + }, + }, + }, + LogService: &ocp.OPAServiceConfig{ + Name: "logs", + URL: "https://log-service/ocp", + Credentials: &ocp.ServiceCredentials{ + Bearer: &ocp.Bearer{ + TokenPath: "/etc/opa/auth/token", + }, + }, + }, + DecisionLogReporting: configv2alpha2.DecisionLogReporting{ + UploadSizeLimitBytes: 1048576, + MinDelaySeconds: 1, + MaxDelaySeconds: 30, + }, + }, + // customConfig sets bundles.authz.service to "s4" + customConfig: map[string]interface{}{ + "bundles": map[string]interface{}{ + "authz": map[string]interface{}{ + "service": "s4", + }, + }, + }, + // opaConfig overrides the same key back to "s5" — must win + opaConfig: map[string]interface{}{ + "bundles": map[string]interface{}{ + "authz": map[string]interface{}{ + "service": "s5", + }, + }, + }, + expectedCMContent: `services: +- name: s3 + url: https://minio/ocp + credentials: + s3_signing: + environment_credentials: {} +- name: logs + url: https://log-service/ocp + credentials: + bearer: + token_path: /etc/opa/auth/token +bundles: + authz: + resource: bundles/system/bundle.tar.gz + service: s5 +decision_logs: + reporting: + upload_size_limit_bytes: 1048576 + min_delay_seconds: 1 + max_delay_seconds: 30 + service: logs + resource_path: /logs +`, + }), + ginkgo.Entry("opaConfig adds keys not in customConfig", test{ + opaconf: ocp.OPAConfig{ + BundleResource: "bundles/system/bundle.tar.gz", + BundleService: &ocp.OPAServiceConfig{ + Name: "s3", + URL: "https://minio/ocp", + Credentials: &ocp.ServiceCredentials{ + S3: &ocp.S3Signing{ + S3EnvironmentCredentials: map[string]ocp.EmptyStruct{}, + }, + }, + }, + LogService: &ocp.OPAServiceConfig{ + Name: "logs", + URL: "https://log-service/ocp", + Credentials: &ocp.ServiceCredentials{ + Bearer: &ocp.Bearer{ + TokenPath: "/etc/opa/auth/token", + }, + }, + }, + DecisionLogReporting: configv2alpha2.DecisionLogReporting{ + UploadSizeLimitBytes: 1048576, + MinDelaySeconds: 1, + MaxDelaySeconds: 30, + }, + }, + customConfig: nil, + opaConfig: map[string]interface{}{ + "caching": map[string]interface{}{ + "inter_query_builtin_cache": map[string]interface{}{ + "max_size_bytes": 10000000, + }, + }, + }, + expectedCMContent: `services: +- name: s3 + url: https://minio/ocp + credentials: + s3_signing: + environment_credentials: {} +- name: logs + url: https://log-service/ocp + credentials: + bearer: + token_path: /etc/opa/auth/token +bundles: + authz: + resource: bundles/system/bundle.tar.gz + service: s3 +decision_logs: + reporting: + upload_size_limit_bytes: 1048576 + min_delay_seconds: 1 + max_delay_seconds: 30 + service: logs + resource_path: /logs +caching: + inter_query_builtin_cache: + max_size_bytes: 10000000 +`, + }), + ) +}) + +// Test that controller-level OPAConfig overlays the generated config but can be +// overridden by legacy customConfig and new opaConfig fields. +var _ = ginkgo.Describe("OPAConfToK8sOPAConfigMap controller opaConfig precedence", func() { + + type test struct { + opaDefaultConfig configv2alpha2.OPAConfig + opaconf ocp.OPAConfig + projectConfig map[string]interface{} + customConfig map[string]interface{} + opaConfig map[string]interface{} + expectedCMContent string + } + + ginkgo.DescribeTable("OPAConfToK8sOPAConfigMap", func(test test) { + cm, err := k8sconv.OPAConfToK8sOPAConfigMapforOCP( + test.opaconf, + test.opaDefaultConfig, + test.projectConfig, + test.customConfig, + test.opaConfig, + logr.Discard()) + + gomega.Expect(err).To(gomega.BeNil()) + + var actualMap, expectedMap map[string]interface{} + actualYAML := cm.Data["opa-conf.yaml"] + expectedYAML := test.expectedCMContent + + err = yaml.Unmarshal([]byte(actualYAML), &actualMap) + gomega.Expect(err).ToNot(gomega.HaveOccurred(), "Failed to unmarshal actual YAML") + + err = yaml.Unmarshal([]byte(expectedYAML), &expectedMap) + gomega.Expect(err).ToNot(gomega.HaveOccurred(), "Failed to unmarshal expected YAML") + + gomega.Expect(actualMap).To(gomega.Equal(expectedMap)) + }, + ginkgo.Entry("controller config overrides generated labels and customConfig can override it", test{ + opaDefaultConfig: configv2alpha2.OPAConfig{ + DecisionLogs: configv2alpha2.DecisionLog{ + RequestContext: configv2alpha2.RequestContext{ + HTTP: configv2alpha2.HTTP{ + Headers: strings.Split("header1,header2", ","), + }, + }, + }, + }, + opaconf: ocp.OPAConfig{ + BundleResource: "bundles/system/bundle.tar.gz", + BundleService: &ocp.OPAServiceConfig{ + Name: "s3", + URL: "https://minio/ocp", + Credentials: &ocp.ServiceCredentials{ + S3: &ocp.S3Signing{ + S3EnvironmentCredentials: map[string]ocp.EmptyStruct{}, + }, + }, + }, + LogService: &ocp.OPAServiceConfig{ + Name: "logs", + URL: "https://log-service/ocp", + Credentials: &ocp.ServiceCredentials{ + Bearer: &ocp.Bearer{ + TokenPath: "/etc/opa/auth/token", + }, + }, + }, + DecisionLogReporting: configv2alpha2.DecisionLogReporting{ + UploadSizeLimitBytes: 1048576, + MinDelaySeconds: 1, + MaxDelaySeconds: 30, + }, + }, + projectConfig: map[string]interface{}{ + "labels": map[string]interface{}{ + "namespace": "project-ns", + "role": "project", + }, + }, + customConfig: map[string]interface{}{ + "labels": map[string]interface{}{ + "namespace": "custom-ns", + }, + }, + expectedCMContent: `services: +- name: s3 + url: https://minio/ocp + credentials: + s3_signing: + environment_credentials: {} +- name: logs + url: https://log-service/ocp + credentials: + bearer: + token_path: /etc/opa/auth/token +bundles: + authz: + resource: bundles/system/bundle.tar.gz + service: s3 +decision_logs: + request_context: + http: + headers: + - header1 + - header2 + reporting: + upload_size_limit_bytes: 1048576 + min_delay_seconds: 1 + max_delay_seconds: 30 + service: logs + resource_path: /logs +labels: + namespace: custom-ns + role: project +`, + }), + ) +}) diff --git a/pkg/opaconfig/opaconfig.go b/pkg/opaconfig/opaconfig.go new file mode 100644 index 00000000..6bc9122e --- /dev/null +++ b/pkg/opaconfig/opaconfig.go @@ -0,0 +1,97 @@ +/* +Copyright (C) 2025 Bankdata (bankdata@bankdata.dk) + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package opaconfig contains helpers for validating and converting OPA config. +package opaconfig + +import ( + "fmt" + "sort" + "strings" + + "gopkg.in/yaml.v2" +) + +var knownTopLevelKeys = map[string]struct{}{ + "services": {}, + "labels": {}, + "discovery": {}, + "bundle": {}, + "bundles": {}, + "decision_logs": {}, + "status": {}, + "plugins": {}, + "keys": {}, + "default_decision": {}, + "default_authorization_decision": {}, + "caching": {}, + "nd_builtin_cache": {}, + "persistence_directory": {}, + "distributed_tracing": {}, + "metrics_export": {}, + "server": {}, + "storage": {}, +} + +// ValidateRaw validates raw OPA config bytes and rejects unknown top-level +// keys. +func ValidateRaw(raw []byte) error { + if len(raw) == 0 { + return nil + } + + var cfg map[string]interface{} + if err := yaml.Unmarshal(raw, &cfg); err != nil { + return fmt.Errorf("could not parse OPA config: %w", err) + } + + if len(cfg) == 0 { + return nil + } + + keys := make([]string, 0) + for key := range cfg { + if _, ok := knownTopLevelKeys[key]; !ok { + keys = append(keys, key) + } + } + if len(keys) == 0 { + return nil + } + + sort.Strings(keys) + return fmt.Errorf("unknown OPA configuration key(s): %s", strings.Join(keys, ", ")) +} + +// ToMap converts a typed OPA config into a map representation that can be +// merged with other YAML-derived maps. +func ToMap(cfg any) (map[string]interface{}, error) { + if cfg == nil { + return nil, nil + } + + bs, err := yaml.Marshal(cfg) + if err != nil { + return nil, fmt.Errorf("could not marshal OPA config: %w", err) + } + + var result map[string]interface{} + if err := yaml.Unmarshal(bs, &result); err != nil { + return nil, fmt.Errorf("could not unmarshal OPA config: %w", err) + } + + return result, nil +} diff --git a/pkg/opaconfig/opaconfig_test.go b/pkg/opaconfig/opaconfig_test.go new file mode 100644 index 00000000..42014385 --- /dev/null +++ b/pkg/opaconfig/opaconfig_test.go @@ -0,0 +1,64 @@ +/* +Copyright (C) 2025 Bankdata (bankdata@bankdata.dk) + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package opaconfig + +import "testing" + +func TestValidateRaw(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + raw string + wantErr bool + }{ + { + name: "valid config", + raw: ` +services: +- name: bundle + url: https://example.invalid +labels: + team: platform +`, + }, + { + name: "unknown top-level key", + raw: ` +servicess: +- name: bundle + url: https://example.invalid +`, + wantErr: true, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + err := ValidateRaw([]byte(tt.raw)) + if tt.wantErr && err == nil { + t.Fatalf("expected an error, got nil") + } + if !tt.wantErr && err != nil { + t.Fatalf("expected no error, got %v", err) + } + }) + } +} From 04cc4644729f4dbe72dada7ee9c1fe37bce6e16b Mon Sep 17 00:00:00 2001 From: bdumpp Date: Tue, 7 Jul 2026 12:57:48 +0200 Subject: [PATCH 2/4] =?UTF-8?q?=F0=9F=90=9B=20Fix=20OPA=20config=20generat?= =?UTF-8?q?ion=20and=20backwards=20compatibility?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Restore customOPAConfig to *runtime.RawExtension so existing System resources with non-standard OPA keys are not rejected on read - Remove ValidateRaw from v1beta1.OPAConfigSpec.UnmarshalJSON; move validation of spec.opa.config to the admission webhook (write-only) - Fix nil pointer panic when legacy BundleServer or DecisionAPIConfig is absent (controller config without legacy OPA section) - Fix ToMap to use encoding/json instead of gopkg.in/yaml.v2 so that json: struct tags and runtime.RawExtension values are handled correctly - Fix scientific notation for large integers (UseNumber + int64 conversion) - Always emit bundles.authz.resource in the base ConfigMap Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: bdumpp --- api/styra/v1beta1/opaconfig_types.go | 12 +- api/styra/v1beta1/system_types.go | 2 +- api/styra/v1beta1/zz_generated.deepcopy.go | 2 +- cmd/main.go | 44 +- .../crd/bases/styra.bankdata.dk_systems.yaml | 73 +--- docs/apis/styra/v1alpha1.md | 2 +- docs/apis/styra/v1beta1.md | 375 +++++++++++++++++- .../controller/styra/system_controller.go | 60 +-- internal/k8sconv/k8sconv.go | 26 +- .../webhook/styra/v1beta1/system_webhook.go | 20 + pkg/opaconfig/opaconfig.go | 37 +- 11 files changed, 493 insertions(+), 160 deletions(-) diff --git a/api/styra/v1beta1/opaconfig_types.go b/api/styra/v1beta1/opaconfig_types.go index 929bb6e5..b8ee244f 100644 --- a/api/styra/v1beta1/opaconfig_types.go +++ b/api/styra/v1beta1/opaconfig_types.go @@ -20,8 +20,6 @@ import ( "encoding/json" "k8s.io/apimachinery/pkg/runtime" - - "github.com/bankdata/styra-controller/pkg/opaconfig" ) // OPAConfigSpec mirrors the official OPA configuration schema. @@ -59,13 +57,11 @@ type OPAStorageConfig struct { Disk *runtime.RawExtension `json:"disk,omitempty"` } -// UnmarshalJSON validates the OPA config against the known schema keys before -// decoding it. +// UnmarshalJSON decodes the OPA config. Validation of keys against the OPA +// schema is intentionally not done here so that existing resources with +// non-standard keys can still be read by the controller. Validation is +// performed on write via the admission webhook. func (c *OPAConfigSpec) UnmarshalJSON(data []byte) error { - if err := opaconfig.ValidateRaw(data); err != nil { - return err - } - type alias OPAConfigSpec var decoded alias if err := json.Unmarshal(data, &decoded); err != nil { diff --git a/api/styra/v1beta1/system_types.go b/api/styra/v1beta1/system_types.go index 71602814..7babe891 100644 --- a/api/styra/v1beta1/system_types.go +++ b/api/styra/v1beta1/system_types.go @@ -55,7 +55,7 @@ type SystemSpec struct { // Use OPA.Config instead, which provides the same capability with runtime // validation of field names against the OPA configuration schema. // If both are set, OPA.Config takes precedence on conflicting keys. - CustomOPAConfig *OPAConfigSpec `json:"customOPAConfig,omitempty"` + CustomOPAConfig *runtime.RawExtension `json:"customOPAConfig,omitempty"` // OPA contains OPA-specific configuration for this system. OPA *SystemOPASpec `json:"opa,omitempty"` diff --git a/api/styra/v1beta1/zz_generated.deepcopy.go b/api/styra/v1beta1/zz_generated.deepcopy.go index 55cb210d..f31f5a71 100644 --- a/api/styra/v1beta1/zz_generated.deepcopy.go +++ b/api/styra/v1beta1/zz_generated.deepcopy.go @@ -572,7 +572,7 @@ func (in *SystemSpec) DeepCopyInto(out *SystemSpec) { } if in.CustomOPAConfig != nil { in, out := &in.CustomOPAConfig, &out.CustomOPAConfig - *out = new(OPAConfigSpec) + *out = new(runtime.RawExtension) (*in).DeepCopyInto(*out) } if in.OPA != nil { diff --git a/cmd/main.go b/cmd/main.go index aa0fc8b3..e7ba3ca6 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -23,6 +23,7 @@ import ( "fmt" "os" "strings" + "time" // Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.) // to ensure that exec-entrypoint and run can make use of them. @@ -110,8 +111,7 @@ func main() { ctrlConfig, err := config.Load(configFiles, scheme) if err != nil { - log.Error(err, "unable to load the config file(s)") - exit(err) + exit(errors.Errorf("unable to load the config file(s): %s", err.Error())) } ctrl.SetLogger(zap.New( @@ -123,8 +123,7 @@ func main() { mgr, err := ctrl.NewManager(restCfg, options) if err != nil { - log.Error(err, "unable to start manager") - exit(err) + exit(errors.Errorf("unable to start manager: %s", err.Error())) } var opaControlPlaneClient ocp.ClientInterface @@ -134,14 +133,12 @@ func main() { err := errors.New( "missing OPA Control Plane configuration: address and token are required", ) - log.Error(err, "unable to start manager") - exit(err) + exit(errors.Errorf("unable to start manager: %s", err.Error())) } if ctrlConfig.OPAControlPlaneConfig.BundleObjectStorage == nil { err := errors.New("missing OPA Control Plane bundle object storage configuration") - log.Error(err, "unable to start manager") - exit(err) + exit(errors.Errorf("unable to start manager: %s", err.Error())) } ocpHostURL := strings.TrimSuffix(ctrlConfig.OPAControlPlaneConfig.Address, "/") @@ -158,7 +155,6 @@ func main() { if err := metrics.Registry.Register(systemReadyMetric); err != nil { err := errors.Wrap(err, "could not register controller_system_status_ready metric") - log.Error(err, err.Error()) exit(err) } @@ -172,7 +168,6 @@ func main() { if err := metrics.Registry.Register(reconcileSegmentTimeMetric); err != nil { err := errors.Wrap(err, "could not register reconcileSegmentTimeMetric") - log.Error(err, err.Error()) exit(err) } @@ -186,7 +181,6 @@ func main() { if err := metrics.Registry.Register(reconcileTimeMetric); err != nil { err := errors.Wrap(err, "could not register reconcileTimeMetric") - log.Error(err, err.Error()) exit(err) } @@ -212,19 +206,16 @@ func main() { ctrlConfig.OPAControlPlaneConfig.LibraryDatasourceChanged) if err = r1.SetupWithManager(mgr, "styra-controller"); err != nil { - log.Error(err, "unable to create controller", "controller", "System") - exit(err) + exit(errors.Errorf("unable to create System controller: %s", err.Error())) } if err = r1.CreateDefaultRequirements(context.Background(), log); err != nil { - log.Error(err, "unable to create default requirements") - exit(err) + exit(errors.Errorf("unable to create default requirements: %s", err.Error())) } if !ctrlConfig.DisableCRDWebhooks { if err = webhookstyrav1beta1.SetupSystemWebhookWithManager(mgr); err != nil { - log.Error(err, "unable to create webhook", "webhook", "System") - os.Exit(1) + exit(errors.Errorf("unable to create System webhook: %s", err.Error())) } } @@ -241,33 +232,30 @@ func main() { ctrlConfig.OPAControlPlaneConfig.LibraryDatasourceChanged) if err = libraryReconciler.SetupWithManager(mgr); err != nil { - log.Error(err, "unable to create controller", "controller", "Library") - os.Exit(1) + exit(errors.Errorf("unable to create Library controller: %s", err.Error())) } if !ctrlConfig.DisableCRDWebhooks { if err = webhookstyrav1alpha1.SetupLibraryWebhookWithManager(mgr); err != nil { - log.Error(err, "unable to create webhook", "webhook", "Library") - os.Exit(1) + exit(errors.Errorf("unable to create Library webhook: %s", err.Error())) } } //+kubebuilder:scaffold:builder if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil { - log.Error(err, "unable to set up health check") - exit(err) + exit(errors.Errorf("unable to set up health check: %s", err.Error())) } if err := mgr.AddReadyzCheck("readyz", healthz.Ping); err != nil { - log.Error(err, "unable to set up ready check") - exit(err) + exit(errors.Errorf("unable to set up ready check: %s", err.Error())) } log.Info("starting manager") if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil { - log.Error(err, "problem running manager") - exit(err) + exit(errors.Errorf("problem running manager: %s", err.Error())) } } -func exit(_ error) { +func exit(err error) { + fmt.Printf("Exiting due to error: %v\n", err) + time.Sleep(10 * time.Second) os.Exit(1) } diff --git a/config/crd/bases/styra.bankdata.dk_systems.yaml b/config/crd/bases/styra.bankdata.dk_systems.yaml index 40063c98..4e17df7d 100644 --- a/config/crd/bases/styra.bankdata.dk_systems.yaml +++ b/config/crd/bases/styra.bankdata.dk_systems.yaml @@ -56,79 +56,8 @@ spec: Use OPA.Config instead, which provides the same capability with runtime validation of field names against the OPA configuration schema. If both are set, OPA.Config takes precedence on conflicting keys. - properties: - bundle: - type: object - x-kubernetes-preserve-unknown-fields: true - bundles: - type: object - x-kubernetes-preserve-unknown-fields: true - caching: - type: object - x-kubernetes-preserve-unknown-fields: true - decision_logs: - type: object - x-kubernetes-preserve-unknown-fields: true - default_authorization_decision: - type: string - default_decision: - type: string - discovery: - type: object - x-kubernetes-preserve-unknown-fields: true - distributed_tracing: - type: object - x-kubernetes-preserve-unknown-fields: true - keys: - type: object - x-kubernetes-preserve-unknown-fields: true - labels: - additionalProperties: - type: string - type: object - metrics_export: - type: object - x-kubernetes-preserve-unknown-fields: true - nd_builtin_cache: - type: boolean - persistence_directory: - type: string - plugins: - additionalProperties: - type: object - x-kubernetes-preserve-unknown-fields: true - type: object - server: - description: OPAServerConfig mirrors OPA's server configuration - section. - properties: - decoding: - type: object - x-kubernetes-preserve-unknown-fields: true - encoding: - type: object - x-kubernetes-preserve-unknown-fields: true - logger_plugin: - type: string - metrics: - type: object - x-kubernetes-preserve-unknown-fields: true - type: object - services: - type: object - x-kubernetes-preserve-unknown-fields: true - status: - type: object - x-kubernetes-preserve-unknown-fields: true - storage: - description: OPAStorageConfig mirrors OPA's storage configuration - section. - properties: - disk: - type: object - x-kubernetes-preserve-unknown-fields: true - type: object type: object + x-kubernetes-preserve-unknown-fields: true datasources: description: Datasources represents a list of datasources to be mounted in the system. diff --git a/docs/apis/styra/v1alpha1.md b/docs/apis/styra/v1alpha1.md index 61b556c2..7097ffe4 100644 --- a/docs/apis/styra/v1alpha1.md +++ b/docs/apis/styra/v1alpha1.md @@ -458,5 +458,5 @@ GitRepo

Generated with gen-crd-api-reference-docs -on git commit 4b70bcc. +on git commit 24c7f90.

diff --git a/docs/apis/styra/v1beta1.md b/docs/apis/styra/v1beta1.md index 0e4a8fe0..5e5c289b 100644 --- a/docs/apis/styra/v1beta1.md +++ b/docs/apis/styra/v1beta1.md @@ -736,6 +736,208 @@ string +

OPAConfigSpec +

+

+(Appears on:SystemOPASpec) +

+
+

OPAConfigSpec mirrors the official OPA configuration schema.

+
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
FieldDescription
+services
+ +k8s.io/apimachinery/pkg/runtime.RawExtension + +		 +
+labels
+ +map[string]string + +		 +
+discovery
+ +k8s.io/apimachinery/pkg/runtime.RawExtension + +		 +
+bundle
+ +k8s.io/apimachinery/pkg/runtime.RawExtension + +		 +
+bundles
+ +k8s.io/apimachinery/pkg/runtime.RawExtension + +		 +
+decision_logs
+ +k8s.io/apimachinery/pkg/runtime.RawExtension + +		 +
+status
+ +k8s.io/apimachinery/pkg/runtime.RawExtension + +		 +
+plugins
+ +map[string]*k8s.io/apimachinery/pkg/runtime.RawExtension + +		 +
+keys
+ +k8s.io/apimachinery/pkg/runtime.RawExtension + +		 +
+default_decision
+ +string + +		 +
+default_authorization_decision
+ +string + +		 +
+caching
+ +k8s.io/apimachinery/pkg/runtime.RawExtension + +		 +
+nd_builtin_cache
+ +bool + +		 +
+persistence_directory
+ +string + +		 +
+distributed_tracing
+ +k8s.io/apimachinery/pkg/runtime.RawExtension + +		 +
+metrics_export
+ +k8s.io/apimachinery/pkg/runtime.RawExtension + +		 +
+server
+ + +OPAServerConfig + + +		 +
+storage
+ + +OPAStorageConfig + + +		 +

OPAConfigStatus

@@ -764,6 +966,92 @@ bool +

OPAServerConfig +

+

+(Appears on:OPAConfigSpec) +

+
+

OPAServerConfig mirrors OPA’s server configuration section.

+
+ + + + + + + + + + + + + + + + + + + + + + + + + +
FieldDescription
+metrics
+ +k8s.io/apimachinery/pkg/runtime.RawExtension + +		 +
+encoding
+ +k8s.io/apimachinery/pkg/runtime.RawExtension + +		 +
+decoding
+ +k8s.io/apimachinery/pkg/runtime.RawExtension + +		 +
+logger_plugin
+ +string + +		 +
+

OPAStorageConfig +

+

+(Appears on:OPAConfigSpec) +

+
+

OPAStorageConfig mirrors OPA’s storage configuration section.

+
+ + + + + + + + + + + + + +
FieldDescription
+disk
+ +k8s.io/apimachinery/pkg/runtime.RawExtension + +		 +

ReasonMapping

@@ -1003,9 +1291,8 @@ DiscoveryOverrides -

DiscoveryOverrides is an OPA config which will take precedence over the -configuration supplied by the OPA discovery API. Configuration set here -will be merged with the configuration supplied by the discovery API.

+

Deprecated: DiscoveryOverrides is unused by the controller and will be +removed in a future version. Use OPA.Config instead.

@@ -1040,8 +1327,24 @@ k8s.io/apimachinery/pkg/runtime.RawExtension -

CustomOPAConfig allows the owner of a System resource to set custom features -without having to extend the Controller

+

Deprecated: CustomOPAConfig allows the owner of a System resource to set +custom OPA configuration that is merged into the generated OPA config. +Use OPA.Config instead, which provides the same capability with runtime +validation of field names against the OPA configuration schema. +If both are set, OPA.Config takes precedence on conflicting keys.

+ + + + +opa
+ + +SystemOPASpec + + + + +

OPA contains OPA-specific configuration for this system.

@@ -1062,6 +1365,41 @@ SystemStatus +

SystemOPASpec +

+

+(Appears on:SystemSpec) +

+
+

SystemOPASpec contains OPA-specific configuration for a System.

+
+ + + + + + + + + + + + + +
FieldDescription
+config
+ + +OPAConfigSpec + + +		 +

Config accepts any valid OPA configuration YAML. The top-level keys are +validated by the OPA configuration schema (see +https://github.com/open-policy-agent/opa/blob/main/v1/config/config.go).

+

Configuration set here is merged on top of the generated OPA config and +takes precedence over the deprecated customOPAConfig field on conflicts.

+

SystemPhase (string alias)

@@ -1179,9 +1517,8 @@ DiscoveryOverrides -

DiscoveryOverrides is an OPA config which will take precedence over the -configuration supplied by the OPA discovery API. Configuration set here -will be merged with the configuration supplied by the discovery API.

+

Deprecated: DiscoveryOverrides is unused by the controller and will be +removed in a future version. Use OPA.Config instead.

@@ -1216,8 +1553,24 @@ k8s.io/apimachinery/pkg/runtime.RawExtension -

CustomOPAConfig allows the owner of a System resource to set custom features -without having to extend the Controller

+

Deprecated: CustomOPAConfig allows the owner of a System resource to set +custom OPA configuration that is merged into the generated OPA config. +Use OPA.Config instead, which provides the same capability with runtime +validation of field names against the OPA configuration schema. +If both are set, OPA.Config takes precedence on conflicting keys.

+ + + + +opa
+ + +SystemOPASpec + + + + +

OPA contains OPA-specific configuration for this system.

@@ -1303,5 +1656,5 @@ System.

Generated with gen-crd-api-reference-docs -on git commit 4b70bcc. +on git commit 24c7f90.

diff --git a/internal/controller/styra/system_controller.go b/internal/controller/styra/system_controller.go index 1e348c19..a968b28e 100644 --- a/internal/controller/styra/system_controller.go +++ b/internal/controller/styra/system_controller.go @@ -423,7 +423,6 @@ func (r *SystemReconciler) reconcileOPAConfigMapForOCP( ) (ctrl.Result, bool, error) { log.Info("Reconciling OPA ConfigMap") - var expectedOPAConfigMap corev1.ConfigMap var projectConfig map[string]interface{} if r.Config.OPAConfig != nil { var err error @@ -458,33 +457,38 @@ func (r *SystemReconciler) reconcileOPAConfigMapForOCP( legacyOPA := r.Config.OPA //nolint:staticcheck legacyBundleServer := legacyOPA.BundleServer legacyDecisionAPIConfig := legacyOPA.DecisionAPIConfig - bundleURL, err := url.JoinPath(legacyBundleServer.URL, legacyBundleServer.Path) - if err != nil { - return ctrl.Result{}, false, ctrlerr.Wrap(err, "Invalid OPA BundleServer URL or path"). - WithEvent(v1beta1.EventErrorConvertOPAConf). - WithSystemCondition(v1beta1.ConditionTypeOPAConfigMapUpdated) - } - bundleServiceCredentials := &ocp.ServiceCredentials{ - S3: &ocp.S3Signing{ - S3EnvironmentCredentials: map[string]ocp.EmptyStruct{}, - }, - } - if legacyBundleServer.TokenPath != "" { - bundleServiceCredentials = &ocp.ServiceCredentials{ - Bearer: &ocp.Bearer{ - TokenPath: legacyBundleServer.TokenPath, + var bundleService *ocp.OPAServiceConfig + if legacyBundleServer != nil { + bundleURL, err := url.JoinPath(legacyBundleServer.URL, legacyBundleServer.Path) + if err != nil { + return ctrl.Result{}, false, ctrlerr.Wrap(err, "Invalid OPA BundleServer URL or path"). + WithEvent(v1beta1.EventErrorConvertOPAConf). + WithSystemCondition(v1beta1.ConditionTypeOPAConfigMapUpdated) + } + bundleServiceCredentials := &ocp.ServiceCredentials{ + S3: &ocp.S3Signing{ + S3EnvironmentCredentials: map[string]ocp.EmptyStruct{}, }, } - } - - opaconf := ocp.OPAConfig{ - BundleService: &ocp.OPAServiceConfig{ + if legacyBundleServer.TokenPath != "" { + bundleServiceCredentials = &ocp.ServiceCredentials{ + Bearer: &ocp.Bearer{ + TokenPath: legacyBundleServer.TokenPath, + }, + } + } + bundleService = &ocp.OPAServiceConfig{ Name: legacyBundleServer.Name, URL: bundleURL, Credentials: bundleServiceCredentials, - }, - LogService: &ocp.OPAServiceConfig{ + } + } + + var logService *ocp.OPAServiceConfig + var decisionLogReporting configv2alpha2.DecisionLogReporting + if legacyDecisionAPIConfig != nil { + logService = &ocp.OPAServiceConfig{ Name: legacyDecisionAPIConfig.Name, URL: legacyDecisionAPIConfig.ServiceURL, Credentials: &ocp.ServiceCredentials{ @@ -492,14 +496,20 @@ func (r *SystemReconciler) reconcileOPAConfigMapForOCP( TokenPath: legacyDecisionAPIConfig.TokenPath, }, }, - }, - DecisionLogReporting: legacyDecisionAPIConfig.Reporting, + } + decisionLogReporting = legacyDecisionAPIConfig.Reporting + } + + opaconf := ocp.OPAConfig{ + BundleService: bundleService, + LogService: logService, + DecisionLogReporting: decisionLogReporting, BundleResource: fmt.Sprintf("bundles/%s/bundle.tar.gz", uniqueName), UniqueName: uniqueName, Namespace: system.Namespace, } - expectedOPAConfigMap, err = k8sconv.OPAConfToK8sOPAConfigMapforOCP( + expectedOPAConfigMap, err := k8sconv.OPAConfToK8sOPAConfigMapforOCP( opaconf, legacyOPA, projectConfig, diff --git a/internal/k8sconv/k8sconv.go b/internal/k8sconv/k8sconv.go index 45c16f79..e892089e 100644 --- a/internal/k8sconv/k8sconv.go +++ b/internal/k8sconv/k8sconv.go @@ -37,8 +37,8 @@ type credentials struct { } type authz struct { - Service string `yaml:"service"` - Resource string `yaml:"resource"` + Service string `yaml:"service,omitempty"` + Resource string `yaml:"resource,omitempty"` Persist bool `yaml:"persist,omitempty"` } @@ -156,18 +156,24 @@ func OPAConfToK8sOPAConfigMapforOCP( } ocpOPAConfigMap := OcpOPAConfigMap{ - Bundles: bundle{ - Authz: authz{ - Service: opaconf.BundleService.Name, - Resource: opaconf.BundleResource, - }, - }, Services: services, Labels: labelsOCP{ UniqueName: opaconf.UniqueName, Namespace: opaconf.Namespace, }, - DecisionLogs: DecisionLogs{ + Bundles: bundle{ + Authz: authz{ + Resource: opaconf.BundleResource, + }, + }, + } + + if opaconf.BundleService != nil { + ocpOPAConfigMap.Bundles.Authz.Service = opaconf.BundleService.Name + } + + if opaconf.LogService != nil { + ocpOPAConfigMap.DecisionLogs = DecisionLogs{ ServiceName: opaconf.LogService.Name, ResourcePath: "/logs", Reporting: &DecisionLogReporting{ @@ -175,7 +181,7 @@ func OPAConfToK8sOPAConfigMapforOCP( MinDelaySeconds: opaconf.DecisionLogReporting.MinDelaySeconds, UploadSizeLimitBytes: opaconf.DecisionLogReporting.UploadSizeLimitBytes, }, - }, + } } if opaDefaultConfig.Metrics.Prometheus.HTTP.Buckets != nil { diff --git a/internal/webhook/styra/v1beta1/system_webhook.go b/internal/webhook/styra/v1beta1/system_webhook.go index 8de8b799..0d73e2d5 100644 --- a/internal/webhook/styra/v1beta1/system_webhook.go +++ b/internal/webhook/styra/v1beta1/system_webhook.go @@ -19,6 +19,7 @@ package v1beta1 import ( "context" + "encoding/json" "sort" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -29,6 +30,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook/admission" styrav1beta1 "github.com/bankdata/styra-controller/api/styra/v1beta1" + "github.com/bankdata/styra-controller/pkg/opaconfig" ) // nolint:all @@ -138,10 +140,28 @@ func validateSystemSpec(s *styrav1beta1.SystemSpec, path *field.Path) field.Erro errs = append(errs, validateDecisionMappings(s, path.Child("decisionMappings"))...) errs = append(errs, validateDatasources(s, path.Child("datasources"))...) + errs = append(errs, validateOPAConfig(s, path)...) return errs } +func validateOPAConfig(s *styrav1beta1.SystemSpec, path *field.Path) field.ErrorList { + if s.OPA == nil || s.OPA.Config == nil { + return nil + } + + raw, err := json.Marshal(s.OPA.Config) + if err != nil { + return field.ErrorList{field.InternalError(path.Child("opa", "config"), err)} + } + + if err := opaconfig.ValidateRaw(raw); err != nil { + return field.ErrorList{field.Invalid(path.Child("opa", "config"), s.OPA.Config, err.Error())} + } + + return nil +} + func validateDecisionMappings(s *styrav1beta1.SystemSpec, path *field.Path) field.ErrorList { var errs field.ErrorList diff --git a/pkg/opaconfig/opaconfig.go b/pkg/opaconfig/opaconfig.go index 6bc9122e..bc8e81be 100644 --- a/pkg/opaconfig/opaconfig.go +++ b/pkg/opaconfig/opaconfig.go @@ -18,6 +18,8 @@ limitations under the License. package opaconfig import ( + "bytes" + "encoding/json" "fmt" "sort" "strings" @@ -83,15 +85,44 @@ func ToMap(cfg any) (map[string]interface{}, error) { return nil, nil } - bs, err := yaml.Marshal(cfg) + bs, err := json.Marshal(cfg) if err != nil { return nil, fmt.Errorf("could not marshal OPA config: %w", err) } + // UseNumber preserves integer values as-is instead of converting them to + // float64, which yaml.Marshal would format as scientific notation for large + // values (e.g. 1048576 → 1.048576e+06). + dec := json.NewDecoder(bytes.NewReader(bs)) + dec.UseNumber() var result map[string]interface{} - if err := yaml.Unmarshal(bs, &result); err != nil { + if err := dec.Decode(&result); err != nil { return nil, fmt.Errorf("could not unmarshal OPA config: %w", err) } - return result, nil + return convertJSONNumbers(result).(map[string]interface{}), nil +} + +// convertJSONNumbers recursively converts json.Number values to int64 or +// float64 so that yaml.Marshal produces human-readable output. +func convertJSONNumbers(v interface{}) interface{} { + switch val := v.(type) { + case json.Number: + if i, err := val.Int64(); err == nil { + return i + } + if f, err := val.Float64(); err == nil { + return f + } + return val.String() + case map[string]interface{}: + for k, vv := range val { + val[k] = convertJSONNumbers(vv) + } + case []interface{}: + for i, vv := range val { + val[i] = convertJSONNumbers(vv) + } + } + return v } From c880e341c790e9d2c87b3240e9c56a420ae3f491 Mon Sep 17 00:00:00 2001 From: bdumpp Date: Tue, 7 Jul 2026 15:36:00 +0200 Subject: [PATCH 3/4] =?UTF-8?q?=F0=9F=94=A8=20Apply=20review=20comments=20?= =?UTF-8?q?and=20fix=20tests=20for=20OPAConfToK8sOPAConfigMapforOCP=20refa?= =?UTF-8?q?ctor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: bdumpp --- .golangci.yml | 4 + api/config/v2alpha2/opaconfig_types.go | 3 +- api/config/v2alpha2/projectconfig_types.go | 10 ++- api/styra/v1beta1/opaconfig_types.go | 3 +- cmd/main.go | 24 +++--- .../controller/styra/system_controller.go | 86 ++++++++++--------- internal/k8sconv/k8sconv.go | 63 +++++++------- internal/k8sconv/k8sconv_test.go | 22 +++-- pkg/ocp/opaconfig.go | 12 ++- pkg/opaconfig/opaconfig_test.go | 1 - 10 files changed, 131 insertions(+), 97 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 338c2d2d..c70ed67b 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -34,3 +34,7 @@ issues: - linters: - lll source: "^//\\+kubebuilder" + - path: "_test\\.go" + linters: + - staticcheck + text: "SA1019" diff --git a/api/config/v2alpha2/opaconfig_types.go b/api/config/v2alpha2/opaconfig_types.go index bf0545c1..1b89510f 100644 --- a/api/config/v2alpha2/opaconfig_types.go +++ b/api/config/v2alpha2/opaconfig_types.go @@ -66,8 +66,7 @@ func (c *OPAConfigSpec) UnmarshalJSON(data []byte) error { return err } - type alias OPAConfigSpec - var decoded alias + var decoded OPAConfigSpec if err := json.Unmarshal(data, &decoded); err != nil { return err } diff --git a/api/config/v2alpha2/projectconfig_types.go b/api/config/v2alpha2/projectconfig_types.go index ca91f409..427a4f21 100644 --- a/api/config/v2alpha2/projectconfig_types.go +++ b/api/config/v2alpha2/projectconfig_types.go @@ -136,6 +136,8 @@ type GitCredentials struct { } // OPAConfig contains default configuration for generated OPA config. +// +// Deprecated: use the typed OPA config schema instead. type OPAConfig struct { DecisionLogs DecisionLog `json:"decisionLogs,omitempty" yaml:"decisionLogs,omitempty"` Metrics MetricsConfig `json:"metrics,omitempty" yaml:"metrics,omitempty"` @@ -173,7 +175,9 @@ type DecisionLog struct { RequestContext RequestContext `json:"requestContext,omitempty"` } -// DecisionAPIConfig contains configuration for decision log dispatch +// DecisionAPIConfig contains configuration for decision log dispatch. +// +// Deprecated: use the typed OPA config schema instead. type DecisionAPIConfig struct { Name string `json:"name,omitempty"` ServiceURL string `json:"serviceUrl,omitempty"` @@ -181,7 +185,9 @@ type DecisionAPIConfig struct { Reporting DecisionLogReporting `json:"reporting,omitempty"` } -// DecisionLogReporting contains configuration for decision log reporting +// DecisionLogReporting contains configuration for decision log reporting. +// +// Deprecated: use the typed OPA config schema instead. type DecisionLogReporting struct { MaxDelaySeconds int `json:"maxDelaySeconds,omitempty" yaml:"maxDelaySeconds,omitempty"` MinDelaySeconds int `json:"minDelaySeconds,omitempty" yaml:"minDelaySeconds,omitempty"` diff --git a/api/styra/v1beta1/opaconfig_types.go b/api/styra/v1beta1/opaconfig_types.go index b8ee244f..affd590c 100644 --- a/api/styra/v1beta1/opaconfig_types.go +++ b/api/styra/v1beta1/opaconfig_types.go @@ -62,8 +62,7 @@ type OPAStorageConfig struct { // non-standard keys can still be read by the controller. Validation is // performed on write via the admission webhook. func (c *OPAConfigSpec) UnmarshalJSON(data []byte) error { - type alias OPAConfigSpec - var decoded alias + var decoded OPAConfigSpec if err := json.Unmarshal(data, &decoded); err != nil { return err } diff --git a/cmd/main.go b/cmd/main.go index e7ba3ca6..42f06862 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -111,7 +111,7 @@ func main() { ctrlConfig, err := config.Load(configFiles, scheme) if err != nil { - exit(errors.Errorf("unable to load the config file(s): %s", err.Error())) + exit(errors.Wrap(err, "unable to load the config file(s)")) } ctrl.SetLogger(zap.New( @@ -123,7 +123,7 @@ func main() { mgr, err := ctrl.NewManager(restCfg, options) if err != nil { - exit(errors.Errorf("unable to start manager: %s", err.Error())) + exit(errors.Wrap(err, "unable to start manager")) } var opaControlPlaneClient ocp.ClientInterface @@ -133,12 +133,12 @@ func main() { err := errors.New( "missing OPA Control Plane configuration: address and token are required", ) - exit(errors.Errorf("unable to start manager: %s", err.Error())) + exit(errors.Wrap(err, "unable to start manager")) } if ctrlConfig.OPAControlPlaneConfig.BundleObjectStorage == nil { err := errors.New("missing OPA Control Plane bundle object storage configuration") - exit(errors.Errorf("unable to start manager: %s", err.Error())) + exit(errors.Wrap(err, "unable to start manager")) } ocpHostURL := strings.TrimSuffix(ctrlConfig.OPAControlPlaneConfig.Address, "/") @@ -206,16 +206,16 @@ func main() { ctrlConfig.OPAControlPlaneConfig.LibraryDatasourceChanged) if err = r1.SetupWithManager(mgr, "styra-controller"); err != nil { - exit(errors.Errorf("unable to create System controller: %s", err.Error())) + exit(errors.Wrap(err, "unable to create System controller")) } if err = r1.CreateDefaultRequirements(context.Background(), log); err != nil { - exit(errors.Errorf("unable to create default requirements: %s", err.Error())) + exit(errors.Wrap(err, "unable to create default requirements")) } if !ctrlConfig.DisableCRDWebhooks { if err = webhookstyrav1beta1.SetupSystemWebhookWithManager(mgr); err != nil { - exit(errors.Errorf("unable to create System webhook: %s", err.Error())) + exit(errors.Wrap(err, "unable to create System webhook")) } } @@ -232,25 +232,25 @@ func main() { ctrlConfig.OPAControlPlaneConfig.LibraryDatasourceChanged) if err = libraryReconciler.SetupWithManager(mgr); err != nil { - exit(errors.Errorf("unable to create Library controller: %s", err.Error())) + exit(errors.Wrap(err, "unable to create Library controller")) } if !ctrlConfig.DisableCRDWebhooks { if err = webhookstyrav1alpha1.SetupLibraryWebhookWithManager(mgr); err != nil { - exit(errors.Errorf("unable to create Library webhook: %s", err.Error())) + exit(errors.Wrap(err, "unable to create Library webhook")) } } //+kubebuilder:scaffold:builder if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil { - exit(errors.Errorf("unable to set up health check: %s", err.Error())) + exit(errors.Wrap(err, "unable to set up health check")) } if err := mgr.AddReadyzCheck("readyz", healthz.Ping); err != nil { - exit(errors.Errorf("unable to set up ready check: %s", err.Error())) + exit(errors.Wrap(err, "unable to set up ready check")) } log.Info("starting manager") if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil { - exit(errors.Errorf("problem running manager: %s", err.Error())) + exit(errors.Wrap(err, "problem running manager")) } } diff --git a/internal/controller/styra/system_controller.go b/internal/controller/styra/system_controller.go index a968b28e..96fc1893 100644 --- a/internal/controller/styra/system_controller.go +++ b/internal/controller/styra/system_controller.go @@ -423,10 +423,10 @@ func (r *SystemReconciler) reconcileOPAConfigMapForOCP( ) (ctrl.Result, bool, error) { log.Info("Reconciling OPA ConfigMap") - var projectConfig map[string]interface{} + var controllerOpaConfig map[string]interface{} if r.Config.OPAConfig != nil { var err error - projectConfig, err = opaconfigutil.ToMap(r.Config.OPAConfig) + controllerOpaConfig, err = opaconfigutil.ToMap(r.Config.OPAConfig) if err != nil { return ctrl.Result{}, false, ctrlerr.Wrap(err, "Could not convert controller OPA config"). WithEvent(v1beta1.EventErrorConvertOPAConf). @@ -434,19 +434,21 @@ func (r *SystemReconciler) reconcileOPAConfigMapForOCP( } } - var customConfig map[string]interface{} + var legacySystemCustomOpaConfig map[string]interface{} if system.Spec.CustomOPAConfig != nil { //nolint:staticcheck var err error - customConfig, err = opaconfigutil.ToMap(system.Spec.CustomOPAConfig) //nolint:staticcheck + legacySystemCustomOpaConfig, err = opaconfigutil.ToMap(system.Spec.CustomOPAConfig) //nolint:staticcheck if err != nil { - return ctrl.Result{}, false, err + return ctrl.Result{}, false, ctrlerr.Wrap(err, "Could not convert legacy controller OPA config"). + WithEvent(v1beta1.EventErrorConvertOPAConf). + WithSystemCondition(v1beta1.ConditionTypeOPAConfigMapUpdated) } } - var opaConfig map[string]interface{} + var systemOpaConfig map[string]interface{} if system.Spec.OPA != nil && system.Spec.OPA.Config != nil { var err error - opaConfig, err = opaconfigutil.ToMap(system.Spec.OPA.Config) + systemOpaConfig, err = opaconfigutil.ToMap(system.Spec.OPA.Config) if err != nil { return ctrl.Result{}, false, ctrlerr.Wrap(err, "Could not convert spec.opa.config"). WithEvent(v1beta1.EventErrorConvertOPAConf). @@ -454,68 +456,68 @@ func (r *SystemReconciler) reconcileOPAConfigMapForOCP( } } - legacyOPA := r.Config.OPA //nolint:staticcheck - legacyBundleServer := legacyOPA.BundleServer - legacyDecisionAPIConfig := legacyOPA.DecisionAPIConfig + legacyControllerOpaConfig := r.Config.OPA //nolint:staticcheck + legacyControllerBundleServer := legacyControllerOpaConfig.BundleServer + legacyControllerDecisionAPIConfig := legacyControllerOpaConfig.DecisionAPIConfig - var bundleService *ocp.OPAServiceConfig - if legacyBundleServer != nil { - bundleURL, err := url.JoinPath(legacyBundleServer.URL, legacyBundleServer.Path) + var legacyBundleService *ocp.OPAServiceConfig //nolint:staticcheck + if legacyControllerBundleServer != nil { + legacyBundleURL, err := url.JoinPath(legacyControllerBundleServer.URL, legacyControllerBundleServer.Path) if err != nil { return ctrl.Result{}, false, ctrlerr.Wrap(err, "Invalid OPA BundleServer URL or path"). WithEvent(v1beta1.EventErrorConvertOPAConf). WithSystemCondition(v1beta1.ConditionTypeOPAConfigMapUpdated) } - bundleServiceCredentials := &ocp.ServiceCredentials{ - S3: &ocp.S3Signing{ + bundleServiceCredentials := &ocp.ServiceCredentials{ //nolint:staticcheck + S3: &ocp.S3Signing{ //nolint:staticcheck S3EnvironmentCredentials: map[string]ocp.EmptyStruct{}, }, } - if legacyBundleServer.TokenPath != "" { - bundleServiceCredentials = &ocp.ServiceCredentials{ - Bearer: &ocp.Bearer{ - TokenPath: legacyBundleServer.TokenPath, + if legacyControllerBundleServer.TokenPath != "" { + bundleServiceCredentials = &ocp.ServiceCredentials{ //nolint:staticcheck + Bearer: &ocp.Bearer{ //nolint:staticcheck + TokenPath: legacyControllerBundleServer.TokenPath, }, } } - bundleService = &ocp.OPAServiceConfig{ - Name: legacyBundleServer.Name, - URL: bundleURL, + legacyBundleService = &ocp.OPAServiceConfig{ //nolint:staticcheck + Name: legacyControllerBundleServer.Name, + URL: legacyBundleURL, Credentials: bundleServiceCredentials, } } - var logService *ocp.OPAServiceConfig - var decisionLogReporting configv2alpha2.DecisionLogReporting - if legacyDecisionAPIConfig != nil { - logService = &ocp.OPAServiceConfig{ - Name: legacyDecisionAPIConfig.Name, - URL: legacyDecisionAPIConfig.ServiceURL, - Credentials: &ocp.ServiceCredentials{ - Bearer: &ocp.Bearer{ - TokenPath: legacyDecisionAPIConfig.TokenPath, + var legacyLogService *ocp.OPAServiceConfig //nolint:staticcheck + var legacyDecisionLogReporting configv2alpha2.DecisionLogReporting //nolint:staticcheck + if legacyControllerDecisionAPIConfig != nil { + legacyLogService = &ocp.OPAServiceConfig{ //nolint:staticcheck + Name: legacyControllerDecisionAPIConfig.Name, + URL: legacyControllerDecisionAPIConfig.ServiceURL, + Credentials: &ocp.ServiceCredentials{ //nolint:staticcheck + Bearer: &ocp.Bearer{ //nolint:staticcheck + TokenPath: legacyControllerDecisionAPIConfig.TokenPath, }, }, } - decisionLogReporting = legacyDecisionAPIConfig.Reporting + legacyDecisionLogReporting = legacyControllerDecisionAPIConfig.Reporting } opaconf := ocp.OPAConfig{ - BundleService: bundleService, - LogService: logService, - DecisionLogReporting: decisionLogReporting, - BundleResource: fmt.Sprintf("bundles/%s/bundle.tar.gz", uniqueName), - UniqueName: uniqueName, - Namespace: system.Namespace, + BundleResource: fmt.Sprintf("bundles/%s/bundle.tar.gz", uniqueName), + UniqueName: uniqueName, + Namespace: system.Namespace, } expectedOPAConfigMap, err := k8sconv.OPAConfToK8sOPAConfigMapforOCP( opaconf, - legacyOPA, - projectConfig, - customConfig, - opaConfig, + legacyControllerOpaConfig, + controllerOpaConfig, + legacySystemCustomOpaConfig, + systemOpaConfig, log, + legacyBundleService, + legacyLogService, + legacyDecisionLogReporting, ) if err != nil { return ctrl.Result{}, false, ctrlerr.Wrap(err, "Could not convert OPA conf to ConfigMap"). diff --git a/internal/k8sconv/k8sconv.go b/internal/k8sconv/k8sconv.go index e892089e..4a5e70b2 100644 --- a/internal/k8sconv/k8sconv.go +++ b/internal/k8sconv/k8sconv.go @@ -101,7 +101,7 @@ type OPAConfigMap struct { // OcpOPAConfigMap represents the structure of the OPA configuration file for OCP type OcpOPAConfigMap struct { - Services []*ocp.OPAServiceConfig `yaml:"services"` + Services []*ocp.OPAServiceConfig `yaml:"services"` //nolint:staticcheck Bundles bundle `yaml:"bundles,omitempty"` DecisionLogs DecisionLogs `yaml:"decision_logs,omitempty"` PersistenceDirectory string `yaml:"persistence_directory,omitempty"` @@ -138,21 +138,25 @@ type HTTPMetricsConfig struct { // OPAConfToK8sOPAConfigMapforOCP creates a ConfigMap for the OPA. // It configures OPA to fetch bundle from MinIO. // OPAConfToK8sOPAConfigMapforOCP merges the information given as input into a ConfigMap for OPA -func OPAConfToK8sOPAConfigMapforOCP( +func OPAConfToK8sOPAConfigMapforOCP( //nolint:staticcheck opaconf ocp.OPAConfig, - opaDefaultConfig configv2alpha2.OPAConfig, - projectConfig map[string]interface{}, - customConfig map[string]interface{}, - opaConfig map[string]interface{}, + legacyControllerOpaConfig configv2alpha2.OPAConfig, //nolint:staticcheck + controllerOpaConfig map[string]interface{}, + legacySystemCustomOpaConfig map[string]interface{}, + systemOpaConfig map[string]interface{}, _ logr.Logger, + legacyBundleService *ocp.OPAServiceConfig, //nolint:staticcheck + legacyLogService *ocp.OPAServiceConfig, //nolint:staticcheck + legacyDecisionLogReporting configv2alpha2.DecisionLogReporting, //nolint:staticcheck ) (corev1.ConfigMap, error) { - var services []*ocp.OPAServiceConfig + var services []*ocp.OPAServiceConfig //nolint:staticcheck - if opaconf.BundleService != nil { - services = append(services, opaconf.BundleService) + if legacyBundleService != nil { + services = append(services, legacyBundleService) } - if opaconf.LogService != nil { - services = append(services, opaconf.LogService) + + if legacyLogService != nil { + services = append(services, legacyLogService) } ocpOPAConfigMap := OcpOPAConfigMap{ @@ -168,28 +172,28 @@ func OPAConfToK8sOPAConfigMapforOCP( }, } - if opaconf.BundleService != nil { - ocpOPAConfigMap.Bundles.Authz.Service = opaconf.BundleService.Name + if legacyBundleService != nil { + ocpOPAConfigMap.Bundles.Authz.Service = legacyBundleService.Name } - if opaconf.LogService != nil { + if legacyLogService != nil { ocpOPAConfigMap.DecisionLogs = DecisionLogs{ - ServiceName: opaconf.LogService.Name, + ServiceName: legacyLogService.Name, ResourcePath: "/logs", Reporting: &DecisionLogReporting{ - MaxDelaySeconds: opaconf.DecisionLogReporting.MaxDelaySeconds, - MinDelaySeconds: opaconf.DecisionLogReporting.MinDelaySeconds, - UploadSizeLimitBytes: opaconf.DecisionLogReporting.UploadSizeLimitBytes, + MaxDelaySeconds: legacyDecisionLogReporting.MaxDelaySeconds, + MinDelaySeconds: legacyDecisionLogReporting.MinDelaySeconds, + UploadSizeLimitBytes: legacyDecisionLogReporting.UploadSizeLimitBytes, }, } } - if opaDefaultConfig.Metrics.Prometheus.HTTP.Buckets != nil { + if legacyControllerOpaConfig.Metrics.Prometheus.HTTP.Buckets != nil { ocpOPAConfigMap.Server = Serverconfig{ Metrics: Metricsconfig{ Prometheus: PrometheusMetricsConfig{ HTTP: HTTPMetricsConfig{ - Buckets: opaDefaultConfig.Metrics.Prometheus.HTTP.Buckets, + Buckets: legacyControllerOpaConfig.Metrics.Prometheus.HTTP.Buckets, }, }, }, @@ -199,15 +203,15 @@ func OPAConfToK8sOPAConfigMapforOCP( } } - if opaDefaultConfig.PersistBundle { - ocpOPAConfigMap.Bundles.Authz.Persist = opaDefaultConfig.PersistBundle - ocpOPAConfigMap.PersistenceDirectory = opaDefaultConfig.PersistBundleDirectory + if legacyControllerOpaConfig.PersistBundle { + ocpOPAConfigMap.Bundles.Authz.Persist = legacyControllerOpaConfig.PersistBundle + ocpOPAConfigMap.PersistenceDirectory = legacyControllerOpaConfig.PersistBundleDirectory } - if opaDefaultConfig.DecisionLogs.RequestContext.HTTP.Headers != nil { + if legacyControllerOpaConfig.DecisionLogs.RequestContext.HTTP.Headers != nil { ocpOPAConfigMap.DecisionLogs.RequestContext = requestContext{ HTTP: http{ - Headers: opaDefaultConfig.DecisionLogs.RequestContext.HTTP.Headers, + Headers: legacyControllerOpaConfig.DecisionLogs.RequestContext.HTTP.Headers, }, } } @@ -217,9 +221,9 @@ func OPAConfToK8sOPAConfigMapforOCP( return corev1.ConfigMap{}, err } - merged := mergeMaps(opaConfigMapMapStringInterface, projectConfig) - merged = mergeMaps(merged, customConfig) - merged = mergeMaps(merged, opaConfig) + merged := mergeMaps(opaConfigMapMapStringInterface, controllerOpaConfig) + merged = mergeMaps(merged, legacySystemCustomOpaConfig) + merged = mergeMaps(merged, systemOpaConfig) res, err := yaml.Marshal(&merged) if err != nil { @@ -250,7 +254,8 @@ func opaConfigMapToMap(cm interface{}) (map[string]interface{}, error) { return opaConfigMapMapStringInterface, nil } -// mergeMaps recursively merges two map[string]interface{} variables +// mergeMaps recursively merges two map[string]interface{} variables. map2 takes precedence +// over map1 in case of key conflicts. func mergeMaps(map1, map2 map[string]interface{}) map[string]interface{} { // TODO: some times, yaml structs have a name as a key and the value under it // but other times, it is a list, where 'name' is one of the fields. diff --git a/internal/k8sconv/k8sconv_test.go b/internal/k8sconv/k8sconv_test.go index d0f58c15..f24ed457 100644 --- a/internal/k8sconv/k8sconv_test.go +++ b/internal/k8sconv/k8sconv_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package k8sconv_test +package k8sconv_test //nolint:staticcheck import ( "strings" @@ -46,7 +46,10 @@ var _ = ginkgo.Describe("OPAConfToK8sOPAConfigMap", func() { nil, test.customConfig, nil, - logr.Discard()) + logr.Discard(), + test.opaconf.BundleService, + test.opaconf.LogService, + test.opaconf.DecisionLogReporting) gomega.Expect(err).To(gomega.BeNil()) @@ -172,7 +175,10 @@ var _ = ginkgo.Describe("OPAConfToK8sOPAConfigMap", func() { nil, test.customConfig, nil, - logr.Discard()) + logr.Discard(), + test.opaconf.BundleService, + test.opaconf.LogService, + test.opaconf.DecisionLogReporting) gomega.Expect(err).To(gomega.BeNil()) @@ -290,7 +296,10 @@ var _ = ginkgo.Describe("OPAConfToK8sOPAConfigMap opaConfig precedence", func() nil, test.customConfig, test.opaConfig, - logr.Discard()) + logr.Discard(), + test.opaconf.BundleService, + test.opaconf.LogService, + test.opaconf.DecisionLogReporting) gomega.Expect(err).To(gomega.BeNil()) @@ -458,7 +467,10 @@ var _ = ginkgo.Describe("OPAConfToK8sOPAConfigMap controller opaConfig precedenc test.projectConfig, test.customConfig, test.opaConfig, - logr.Discard()) + logr.Discard(), + test.opaconf.BundleService, + test.opaconf.LogService, + test.opaconf.DecisionLogReporting) gomega.Expect(err).To(gomega.BeNil()) diff --git a/pkg/ocp/opaconfig.go b/pkg/ocp/opaconfig.go index 2b228691..54046552 100644 --- a/pkg/ocp/opaconfig.go +++ b/pkg/ocp/opaconfig.go @@ -27,10 +27,12 @@ type OPAConfig struct { UniqueName string Namespace string BundleResource string - DecisionLogReporting configv2alpha2.DecisionLogReporting + DecisionLogReporting configv2alpha2.DecisionLogReporting //nolint:staticcheck } -// OPAServiceConfig defines a services added to the OPAs' config files. +// OPAServiceConfig defines services added to the OPAs' config files. +// +// Deprecated: use OPAServiceConfigV2 instead. type OPAServiceConfig struct { Name string `json:"name" yaml:"name"` Credentials *ServiceCredentials `json:"credentials" yaml:"credentials"` @@ -39,12 +41,16 @@ type OPAServiceConfig struct { } // ServiceCredentials defines the structure for service credentials. +// +// Deprecated: use the equivalent in a non-deprecated type. type ServiceCredentials struct { Bearer *Bearer `json:"bearer,omitempty" yaml:"bearer,omitempty"` S3 *S3Signing `json:"s3_signing,omitempty" yaml:"s3_signing,omitempty"` } // S3Signing defines the structure for S3 signing configuration. +// +// Deprecated: use the equivalent in a non-deprecated type. type S3Signing struct { S3EnvironmentCredentials map[string]EmptyStruct `json:"environment_credentials" yaml:"environment_credentials"` } @@ -53,6 +59,8 @@ type S3Signing struct { type EmptyStruct struct{} // Bearer defines the structure for bearer token credentials. +// +// Deprecated: use the equivalent in a non-deprecated type. type Bearer struct { TokenPath string `json:"token_path" yaml:"token_path"` } diff --git a/pkg/opaconfig/opaconfig_test.go b/pkg/opaconfig/opaconfig_test.go index 42014385..b0d2b9c7 100644 --- a/pkg/opaconfig/opaconfig_test.go +++ b/pkg/opaconfig/opaconfig_test.go @@ -48,7 +48,6 @@ servicess: } for _, tt := range tests { - tt := tt t.Run(tt.name, func(t *testing.T) { t.Parallel() From 03fc50e4f05c50c002e33374915d2a73847c3b86 Mon Sep 17 00:00:00 2001 From: bdumpp Date: Wed, 8 Jul 2026 10:11:13 +0200 Subject: [PATCH 4/4] =?UTF-8?q?=F0=9F=90=9B=20Fix=20infinite=20recursion?= =?UTF-8?q?=20in=20OPAConfigSpec.UnmarshalJSON?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- api/config/v2alpha2/opaconfig_types.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/api/config/v2alpha2/opaconfig_types.go b/api/config/v2alpha2/opaconfig_types.go index 1b89510f..324eb81d 100644 --- a/api/config/v2alpha2/opaconfig_types.go +++ b/api/config/v2alpha2/opaconfig_types.go @@ -66,7 +66,10 @@ func (c *OPAConfigSpec) UnmarshalJSON(data []byte) error { return err } - var decoded OPAConfigSpec + // Use a local alias to avoid infinite recursion: json.Unmarshal on the + // alias type uses the default struct decoder, bypassing this method. + type alias OPAConfigSpec + var decoded alias if err := json.Unmarshal(data, &decoded); err != nil { return err }