From cfeceaae5404cfb243e63f831546e9775f5116aa Mon Sep 17 00:00:00 2001 From: hugoShaka Date: Fri, 1 Nov 2024 15:19:13 -0400 Subject: [PATCH] operator: fix oidc conenctor max age --- integrations/operator/Makefile | 4 ++ .../apis/resources/v3/oidcconnector_types.go | 38 +++++++++++++++- .../resources/v3/oidcconnector_types_test.go | 43 +++++++++++++++++++ .../oidc_connector_controller_test.go | 2 + 4 files changed, 86 insertions(+), 1 deletion(-) diff --git a/integrations/operator/Makefile b/integrations/operator/Makefile index 145a7c97dd9b1..d57f12e166a26 100644 --- a/integrations/operator/Makefile +++ b/integrations/operator/Makefile @@ -139,6 +139,10 @@ test: export KUBEBUILDER_ASSETS=$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p test: go test ./... -coverprofile cover.out +.PHONY: echo-kubebuilder-assets +echo-kubebuilder-assets: + @echo KUBEBUILDER_ASSETS=$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path) + .PHONY: crdgen-test crdgen-test: ## Run crdgen tests. make -C crdgen test diff --git a/integrations/operator/apis/resources/v3/oidcconnector_types.go b/integrations/operator/apis/resources/v3/oidcconnector_types.go index 3eedf1d9b5264..d23f25f4fe6d6 100644 --- a/integrations/operator/apis/resources/v3/oidcconnector_types.go +++ b/integrations/operator/apis/resources/v3/oidcconnector_types.go @@ -21,6 +21,7 @@ package v3 import ( "encoding/json" + "github.com/gravitational/trace" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/gravitational/teleport/api/types" @@ -97,14 +98,49 @@ func (spec *TeleportOIDCConnectorSpec) DeepCopyInto(out *TeleportOIDCConnectorSp } } +// Custom json.Marshaller and json.Unmarshaler are here to cope with inconsistencies between our CRD and go types. +// They are invoked when the kubernetes client converts the unstructured object into a typed resource. +// We have two inconsistencies: +// - the utils.Strings typr that marshals inconsistently: single elements are strings, multiple elements are lists +// - the max_age setting which is an embedded pointer to another single-value message, which breaks JSON parsing + // MarshalJSON serializes a spec into a JSON string func (spec TeleportOIDCConnectorSpec) MarshalJSON() ([]byte, error) { type Alias TeleportOIDCConnectorSpec + + var maxAge types.Duration + if spec.MaxAge != nil { + maxAge = spec.MaxAge.Value + } + return json.Marshal(&struct { - RedirectURLs []string `json:"redirect_url"` + RedirectURLs []string `json:"redirect_url,omitempty"` + MaxAge types.Duration `json:"max_age,omitempty"` Alias }{ RedirectURLs: spec.RedirectURLs, + MaxAge: maxAge, Alias: (Alias)(spec), }) } + +// UnmarshalJSON serializes a JSON string into a spec. This override is required to deal with the +// MaxAge field which is special case because it' an object embedded into the spec. +func (spec *TeleportOIDCConnectorSpec) UnmarshalJSON(data []byte) error { + *spec = *new(TeleportOIDCConnectorSpec) + type Alias TeleportOIDCConnectorSpec + + temp := &struct { + MaxAge types.Duration `json:"max_age"` + *Alias + }{ + Alias: (*Alias)(spec), + } + if err := json.Unmarshal(data, &temp); err != nil { + return trace.Wrap(err, "unmarshalling custom teleport oidc connector spec") + } + if temp.MaxAge != 0 { + spec.MaxAge = &types.MaxAge{Value: temp.MaxAge} + } + return nil +} diff --git a/integrations/operator/apis/resources/v3/oidcconnector_types_test.go b/integrations/operator/apis/resources/v3/oidcconnector_types_test.go index c6abb53659989..5c511d5d82905 100644 --- a/integrations/operator/apis/resources/v3/oidcconnector_types_test.go +++ b/integrations/operator/apis/resources/v3/oidcconnector_types_test.go @@ -21,9 +21,11 @@ package v3 import ( "encoding/json" "testing" + "time" "github.com/stretchr/testify/require" + "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/api/types/wrappers" ) @@ -50,6 +52,11 @@ func TestTeleportOIDCConnectorSpec_MarshalJSON(t *testing.T) { TeleportOIDCConnectorSpec{RedirectURLs: wrappers.Strings{"foo", "bar"}}, `{"redirect_url":["foo","bar"],"issuer_url":"","client_id":"","client_secret":""}`, }, + { + "MaxAge", + TeleportOIDCConnectorSpec{MaxAge: &types.MaxAge{Value: types.Duration(time.Hour)}}, + `{"max_age":"1h0m0s","issuer_url":"","client_id":"","client_secret":""}`, + }, } for _, tc := range tests { tc := tc @@ -60,3 +67,39 @@ func TestTeleportOIDCConnectorSpec_MarshalJSON(t *testing.T) { }) } } +func TestTeleportOIDCConnectorSpec_UnmarshalJSON(t *testing.T) { + tests := []struct { + name string + expectedSpec TeleportOIDCConnectorSpec + inputJSON string + }{ + { + "Empty string", + TeleportOIDCConnectorSpec{RedirectURLs: wrappers.Strings{""}}, + `{"redirect_url":[""],"issuer_url":"","client_id":"","client_secret":""}`, + }, + { + "Single string", + TeleportOIDCConnectorSpec{RedirectURLs: wrappers.Strings{"foo"}}, + `{"redirect_url":["foo"],"issuer_url":"","client_id":"","client_secret":""}`, + }, + { + "Multiple strings", + TeleportOIDCConnectorSpec{RedirectURLs: wrappers.Strings{"foo", "bar"}}, + `{"redirect_url":["foo","bar"],"issuer_url":"","client_id":"","client_secret":""}`, + }, + { + "MaxAge", + TeleportOIDCConnectorSpec{MaxAge: &types.MaxAge{Value: types.Duration(time.Hour)}}, + `{"max_age":"1h0m0s","issuer_url":"","client_id":"","client_secret":""}`, + }, + } + for _, tc := range tests { + tc := tc + t.Run(tc.name, func(t *testing.T) { + var spec TeleportOIDCConnectorSpec + require.NoError(t, json.Unmarshal([]byte(tc.inputJSON), &spec)) + require.Equal(t, tc.expectedSpec, spec) + }) + } +} diff --git a/integrations/operator/controllers/resources/oidc_connector_controller_test.go b/integrations/operator/controllers/resources/oidc_connector_controller_test.go index 35228bc8188f7..39359c2704967 100644 --- a/integrations/operator/controllers/resources/oidc_connector_controller_test.go +++ b/integrations/operator/controllers/resources/oidc_connector_controller_test.go @@ -21,6 +21,7 @@ package resources_test import ( "context" "testing" + "time" "github.com/google/go-cmp/cmp" "github.com/gravitational/trace" @@ -46,6 +47,7 @@ var oidcSpec = types.OIDCConnectorSpecV3{ Roles: []string{"roleA"}, }}, RedirectURLs: []string{"https://redirect"}, + MaxAge: &types.MaxAge{Value: types.Duration(time.Hour)}, } type oidcTestingPrimitives struct {