Skip to content

Commit

Permalink
operator: fix oidc conenctor max age (#48316)
Browse files Browse the repository at this point in the history
  • Loading branch information
hugoShaka authored Nov 4, 2024
1 parent e4b5f70 commit 3e74e6d
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 1 deletion.
4 changes: 4 additions & 0 deletions integrations/operator/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
38 changes: 37 additions & 1 deletion integrations/operator/apis/resources/v3/oidcconnector_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -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
Expand All @@ -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)
})
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ package resources_test
import (
"context"
"testing"
"time"

"github.com/google/go-cmp/cmp"
"github.com/gravitational/trace"
Expand All @@ -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 {
Expand Down

0 comments on commit 3e74e6d

Please sign in to comment.