diff --git a/internal/adm/idp/create.go b/internal/adm/idp/create.go new file mode 100644 index 0000000..0133441 --- /dev/null +++ b/internal/adm/idp/create.go @@ -0,0 +1,110 @@ +/* +Copyright 2026 The Butler Authors. + +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 idp + +import ( + "context" + "fmt" + "os" + + "github.com/butlerdotdev/butler/internal/common/client" + "github.com/butlerdotdev/butler/internal/common/log" + "github.com/spf13/cobra" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "sigs.k8s.io/yaml" +) + +func newCreateCmd(logger *log.Logger) *cobra.Command { + var ( + fromFile string + kubeconfig string + ) + + cmd := &cobra.Command{ + Use: "create NAME", + Short: "Create an identity provider", + Long: `Create an IdentityProvider resource from a YAML file. + +The YAML file should contain a complete IdentityProvider manifest including +apiVersion, kind, metadata, and spec. The OAuth2 client secret is supplied by +reference: spec.oidc.clientSecretRef must point at a Secret you create +separately (for example with kubectl create secret). This matches +'provider create --from-file'. + +The file's spec can carry any IdentityProvider field, including advanced +options the console form does not expose (spec.oidc.insecureSkipVerify and +spec.oidc.googleWorkspace). + +If the file carries metadata.name, it must match NAME. + +Examples: + # Create from a YAML file + butleradm idp create okta --from-file okta.yaml`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + kubeContext, _ := cmd.Flags().GetString("context") + return runCreate(cmd.Context(), logger, args[0], fromFile, kubeconfig, kubeContext) + }, + } + + cmd.Flags().StringVar(&fromFile, "from-file", "", "path to a YAML file containing the IdentityProvider manifest (required)") + cmd.Flags().StringVar(&kubeconfig, "kubeconfig", "", "path to management cluster kubeconfig") + + _ = cmd.MarkFlagRequired("from-file") + + return cmd +} + +func runCreate(ctx context.Context, logger *log.Logger, name, fromFile, kubeconfigPath, kubeContext string) error { + data, err := os.ReadFile(fromFile) + if err != nil { + return fmt.Errorf("reading file %s: %w", fromFile, err) + } + + obj := &unstructured.Unstructured{} + if err := yaml.Unmarshal(data, &obj.Object); err != nil { + return fmt.Errorf("parsing YAML from %s: %w", fromFile, err) + } + + if kind := obj.GetKind(); kind != "" && kind != "IdentityProvider" { + return fmt.Errorf("expected kind IdentityProvider, got %s", kind) + } + if fileName := obj.GetName(); fileName != "" && fileName != name { + return fmt.Errorf("metadata.name %q in %s does not match NAME %q", fileName, fromFile, name) + } + + if obj.GetAPIVersion() == "" { + obj.SetAPIVersion("butler.butlerlabs.dev/v1alpha1") + } + obj.SetKind("IdentityProvider") + obj.SetName(name) + + c, err := client.New(kubeconfigPath, kubeContext) + if err != nil { + return fmt.Errorf("connecting to management cluster: %w", err) + } + + // IdentityProvider is cluster-scoped: no namespace. + _, err = c.Dynamic.Resource(client.IdentityProviderGVR).Create(ctx, obj, metav1.CreateOptions{}) + if err != nil { + return fmt.Errorf("creating IdentityProvider: %w", err) + } + + logger.Success("identity provider created", "name", name) + return nil +} diff --git a/internal/adm/idp/discovery.go b/internal/adm/idp/discovery.go new file mode 100644 index 0000000..64e14f8 --- /dev/null +++ b/internal/adm/idp/discovery.go @@ -0,0 +1,197 @@ +/* +Copyright 2026 The Butler Authors. + +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 idp + +import ( + "context" + "errors" + "fmt" + "io" + "os" + + "github.com/butlerdotdev/butler/internal/common/log" + "github.com/butlerdotdev/butler/internal/common/output" + "github.com/butlerdotdev/butler/internal/common/serverhttp" + "github.com/spf13/cobra" +) + +// testDiscoveryRequest mirrors butler-server's TestDiscoveryRequest +// (POST /api/admin/identity-providers/test). +type testDiscoveryRequest struct { + IssuerURL string `json:"issuerURL"` +} + +// testDiscoveryResponse mirrors butler-server's TestDiscoveryResponse. Both +// the test (issuer URL) and validate (existing IdP) endpoints return it. The +// underlying probe is a read-only OIDC discovery fetch, so neither verb +// mutates anything. +type testDiscoveryResponse struct { + Valid bool `json:"valid"` + Message string `json:"message"` + AuthorizationEndpoint string `json:"authorizationEndpoint,omitempty"` + TokenEndpoint string `json:"tokenEndpoint,omitempty"` + UserInfoEndpoint string `json:"userInfoEndpoint,omitempty"` + JWKSURI string `json:"jwksURI,omitempty"` +} + +// --- test --- + +func newTestCmd(_ *log.Logger) *cobra.Command { + var ( + issuerURL string + outputFormat string + ) + + cmd := &cobra.Command{ + Use: "test", + Short: "Test OIDC discovery for an issuer URL", + Long: `Probe an OIDC issuer's discovery document before creating an identity +provider. The server fetches the issuer's .well-known/openid-configuration and +reports whether discovery succeeds and which endpoints it found. + +This is a read-only probe: it does not create or change any resource. + +Examples: + butleradm idp test --issuer-url https://accounts.google.com + butleradm idp test --issuer-url https://example.okta.com -o json`, + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, args []string) error { + return runTest(cmd.Context(), os.Stdout, issuerURL, outputFormat) + }, + } + + cmd.Flags().StringVar(&issuerURL, "issuer-url", "", "OIDC issuer URL to probe (required)") + cmd.Flags().StringVarP(&outputFormat, "output", "o", "", "output format (table, json, yaml)") + _ = cmd.MarkFlagRequired("issuer-url") + + return cmd +} + +func runTest(ctx context.Context, out io.Writer, issuerURL, outputFormat string) error { + sh, err := serverhttp.New() + if err != nil { + return err + } + var res testDiscoveryResponse + if err := sh.Post(ctx, "/api/admin/identity-providers/test", testDiscoveryRequest{IssuerURL: issuerURL}, &res); err != nil { + return translateDiscoveryError(err) + } + return renderDiscovery(out, res, outputFormat) +} + +// --- validate --- + +func newValidateCmd(_ *log.Logger) *cobra.Command { + var outputFormat string + + cmd := &cobra.Command{ + Use: "validate NAME", + Short: "Test OIDC discovery for an existing identity provider", + Long: `Probe the OIDC discovery document of an existing identity provider, using +the issuer URL already configured on it. Reports whether discovery succeeds and +which endpoints it found. + +This is a read-only probe: it does not change the identity provider. + +Examples: + butleradm idp validate google-workspace + butleradm idp validate okta -o json`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + return runValidate(cmd.Context(), os.Stdout, args[0], outputFormat) + }, + } + + cmd.Flags().StringVarP(&outputFormat, "output", "o", "", "output format (table, json, yaml)") + + return cmd +} + +func runValidate(ctx context.Context, out io.Writer, name, outputFormat string) error { + sh, err := serverhttp.New() + if err != nil { + return err + } + path := fmt.Sprintf("/api/admin/identity-providers/%s/validate", name) + var res testDiscoveryResponse + if err := sh.Post(ctx, path, struct{}{}, &res); err != nil { + return translateDiscoveryError(err) + } + return renderDiscovery(out, res, outputFormat) +} + +// --- shared rendering + error translation --- + +func renderDiscovery(out io.Writer, res testDiscoveryResponse, outputFormat string) error { + switch outputFormat { + case "json": + return output.PrintJSON(out, res) + case "yaml": + return output.PrintYAML(out, res) + case "", "table": + return printDiscovery(out, res) + default: + return fmt.Errorf("unsupported output format %q (use table, json or yaml)", outputFormat) + } +} + +func printDiscovery(out io.Writer, res testDiscoveryResponse) error { + status := "INVALID" + if res.Valid { + status = "VALID" + } + fmt.Fprintf(out, "Discovery: %s\n", status) + if res.Message != "" { + fmt.Fprintf(out, "Message: %s\n", res.Message) + } + if !res.Valid { + return nil + } + t := output.NewTable(out, "ENDPOINT", "URL") + if res.AuthorizationEndpoint != "" { + t.AddRow("authorization", res.AuthorizationEndpoint) + } + if res.TokenEndpoint != "" { + t.AddRow("token", res.TokenEndpoint) + } + if res.UserInfoEndpoint != "" { + t.AddRow("userinfo", res.UserInfoEndpoint) + } + if res.JWKSURI != "" { + t.AddRow("jwks", res.JWKSURI) + } + return t.Flush() +} + +func translateDiscoveryError(err error) error { + if errors.Is(err, serverhttp.ErrSessionExpired) { + fmt.Fprintln(os.Stderr, "Butler session expired. Run 'butleradm login' to re-authenticate.") + return err + } + var se *serverhttp.ServerError + if errors.As(err, &se) { + switch { + case se.IsForbidden(): + return fmt.Errorf("forbidden (identity provider operations require platform admin): %s", se.Message) + case se.IsNotFound(): + return fmt.Errorf("not found: %s", se.Message) + case se.IsBadRequest(): + return fmt.Errorf("invalid request: %s", se.Message) + } + } + return err +} diff --git a/internal/adm/idp/idp.go b/internal/adm/idp/idp.go index 1004842..a623161 100644 --- a/internal/adm/idp/idp.go +++ b/internal/adm/idp/idp.go @@ -47,6 +47,8 @@ Google Workspace, Microsoft Entra ID, Okta, or generic OIDC providers. Examples: butleradm idp list butleradm idp get google-workspace + butleradm idp create okta --from-file okta.yaml + butleradm idp test --issuer-url https://accounts.google.com butleradm idp delete old-provider`, PersistentPreRun: func(cmd *cobra.Command, args []string) { auth.WarnIfUnauthenticated() @@ -55,8 +57,14 @@ Examples: cmd.AddCommand(newListCmd(logger)) cmd.AddCommand(newGetCmd(logger)) + cmd.AddCommand(newCreateCmd(logger)) + cmd.AddCommand(newUpdateCmd(logger)) cmd.AddCommand(newDeleteCmd(logger)) + // OIDC discovery probes (P1 #6): non-mutating pre-create and existing-IdP checks. + cmd.AddCommand(newTestCmd(logger)) + cmd.AddCommand(newValidateCmd(logger)) + return cmd } @@ -113,7 +121,7 @@ func runList(ctx context.Context, logger *log.Logger, kubeconfigPath, kubeContex for _, idp := range list.Items { name := idp.GetName() idpType := client.GetNestedString(idp.Object, "spec", "type") - issuer := client.GetNestedString(idp.Object, "spec", "issuerURL") + issuer := client.GetNestedString(idp.Object, "spec", "oidc", "issuerURL") phase := client.GetNestedString(idp.Object, "status", "phase") age := output.FormatAge(idp.GetCreationTimestamp().Time) @@ -187,14 +195,14 @@ func runGet(ctx context.Context, logger *log.Logger, name, kubeconfigPath, kubeC // Human-readable detail view idpType := client.GetNestedString(idp.Object, "spec", "type") - issuer := client.GetNestedString(idp.Object, "spec", "issuerURL") - clientID := client.GetNestedString(idp.Object, "spec", "clientID") + issuer := client.GetNestedString(idp.Object, "spec", "oidc", "issuerURL") + clientID := client.GetNestedString(idp.Object, "spec", "oidc", "clientID") phase := client.GetNestedString(idp.Object, "status", "phase") message := client.GetNestedString(idp.Object, "status", "message") age := output.FormatAge(idp.GetCreationTimestamp().Time) // Extract scopes - scopesRaw, _, _ := unstructuredNestedSlice(idp.Object, "spec", "scopes") + scopesRaw, _, _ := unstructuredNestedSlice(idp.Object, "spec", "oidc", "scopes") var scopes []string for _, s := range scopesRaw { if str, ok := s.(string); ok { @@ -203,8 +211,8 @@ func runGet(ctx context.Context, logger *log.Logger, name, kubeconfigPath, kubeC } // Extract claim mappings - emailClaim := client.GetNestedString(idp.Object, "spec", "claims", "email") - groupsClaim := client.GetNestedString(idp.Object, "spec", "claims", "groups") + emailClaim := client.GetNestedString(idp.Object, "spec", "oidc", "emailClaim") + groupsClaim := client.GetNestedString(idp.Object, "spec", "oidc", "groupsClaim") fmt.Printf("Name: %s\n", idp.GetName()) fmt.Printf("Type: %s\n", idpType) diff --git a/internal/adm/idp/idp_p16_test.go b/internal/adm/idp/idp_p16_test.go new file mode 100644 index 0000000..8d1bfeb --- /dev/null +++ b/internal/adm/idp/idp_p16_test.go @@ -0,0 +1,129 @@ +/* +Copyright 2026 The Butler Authors. + +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 idp + +import ( + "bytes" + "net/http" + "strings" + "testing" + + "github.com/butlerdotdev/butler/internal/common/serverhttp" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" +) + +// TestMergeIdPSpec is the load-bearing update correctness test. A spec update +// must replace spec but PRESERVE the existing status (controller-managed +// conditions) and resourceVersion, so it never clobbers status and the Update +// carries the resourceVersion the API server requires. +func TestMergeIdPSpec(t *testing.T) { + existing := &unstructured.Unstructured{Object: map[string]interface{}{ + "apiVersion": "butler.butlerlabs.dev/v1alpha1", + "kind": "IdentityProvider", + "metadata": map[string]interface{}{ + "name": "okta", + "resourceVersion": "98765", + }, + "spec": map[string]interface{}{ + "type": "oidc", + "oidc": map[string]interface{}{"issuerURL": "https://old.okta.com"}, + }, + "status": map[string]interface{}{ + "conditions": []interface{}{ + map[string]interface{}{"type": "Ready", "status": "True"}, + }, + }, + }} + + newSpec := map[string]interface{}{ + "type": "oidc", + "oidc": map[string]interface{}{"issuerURL": "https://new.okta.com"}, + } + + if err := mergeIdPSpec(existing, newSpec); err != nil { + t.Fatal(err) + } + + // Spec replaced. + got, _, _ := unstructured.NestedString(existing.Object, "spec", "oidc", "issuerURL") + if got != "https://new.okta.com" { + t.Errorf("spec not updated: issuerURL = %q, want https://new.okta.com", got) + } + + // resourceVersion preserved (Update needs it for optimistic concurrency). + if rv := existing.GetResourceVersion(); rv != "98765" { + t.Errorf("resourceVersion clobbered: got %q, want 98765", rv) + } + + // Status conditions preserved untouched. + conds, found, _ := unstructured.NestedSlice(existing.Object, "status", "conditions") + if !found || len(conds) != 1 { + t.Fatalf("status.conditions clobbered: found=%v len=%d (want 1)", found, len(conds)) + } + cond, _ := conds[0].(map[string]interface{}) + if cond["type"] != "Ready" || cond["status"] != "True" { + t.Errorf("status condition altered: %v", cond) + } +} + +func TestMergeIdPSpecEmptyExisting(t *testing.T) { + if err := mergeIdPSpec(&unstructured.Unstructured{}, map[string]interface{}{"type": "oidc"}); err == nil { + t.Error("expected error merging into empty object, got nil") + } +} + +func TestPrintDiscovery(t *testing.T) { + var out bytes.Buffer + res := testDiscoveryResponse{ + Valid: true, + Message: "OIDC discovery successful", + AuthorizationEndpoint: "https://idp/authorize", + TokenEndpoint: "https://idp/token", + } + if err := printDiscovery(&out, res); err != nil { + t.Fatal(err) + } + for _, want := range []string{"VALID", "https://idp/authorize", "https://idp/token"} { + if !strings.Contains(out.String(), want) { + t.Errorf("discovery output missing %q:\n%s", want, out.String()) + } + } + + var bad bytes.Buffer + _ = printDiscovery(&bad, testDiscoveryResponse{Valid: false, Message: "OIDC discovery failed: no such host"}) + if !strings.Contains(bad.String(), "INVALID") || !strings.Contains(bad.String(), "no such host") { + t.Errorf("invalid-discovery output should report INVALID + reason:\n%s", bad.String()) + } +} + +func TestTranslateDiscoveryError(t *testing.T) { + // 403 names the platform-admin requirement. + forbidden := &serverhttp.ServerError{StatusCode: http.StatusForbidden, Message: "admin required"} + if got := translateDiscoveryError(forbidden).Error(); !strings.Contains(got, "platform admin") { + t.Errorf("403 should mention platform admin: %s", got) + } + // 404 maps to not found. + nf := &serverhttp.ServerError{StatusCode: http.StatusNotFound, Message: "identity provider \"ghost\" not found"} + if got := translateDiscoveryError(nf).Error(); !strings.Contains(got, "not found") { + t.Errorf("404 should map to not found: %s", got) + } + // 400 maps to invalid request. + br := &serverhttp.ServerError{StatusCode: http.StatusBadRequest, Message: "issuerURL is required"} + if got := translateDiscoveryError(br).Error(); !strings.Contains(got, "invalid request") { + t.Errorf("400 should map to invalid request: %s", got) + } +} diff --git a/internal/adm/idp/update.go b/internal/adm/idp/update.go new file mode 100644 index 0000000..d4ccb54 --- /dev/null +++ b/internal/adm/idp/update.go @@ -0,0 +1,129 @@ +/* +Copyright 2026 The Butler Authors. + +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 idp + +import ( + "context" + "fmt" + "os" + + "github.com/butlerdotdev/butler/internal/common/client" + "github.com/butlerdotdev/butler/internal/common/log" + "github.com/spf13/cobra" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "sigs.k8s.io/yaml" +) + +func newUpdateCmd(logger *log.Logger) *cobra.Command { + var ( + fromFile string + kubeconfig string + ) + + cmd := &cobra.Command{ + Use: "update NAME", + Short: "Update an identity provider", + Long: `Update the spec of an existing IdentityProvider from a YAML file. + +The YAML file supplies the new spec. Only spec is applied; the existing +status and resourceVersion are preserved, so a spec edit never clobbers the +controller-managed status conditions. This mirrors 'idp create --from-file' +and 'provider update --from-file'. + +If the file carries metadata.name, it must match NAME. + +Examples: + # Update an identity provider from a YAML file + butleradm idp update okta --from-file okta-updated.yaml`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + kubeContext, _ := cmd.Flags().GetString("context") + return runUpdate(cmd.Context(), logger, args[0], fromFile, kubeconfig, kubeContext) + }, + } + + cmd.Flags().StringVar(&fromFile, "from-file", "", "path to a YAML file containing the new IdentityProvider spec (required)") + cmd.Flags().StringVar(&kubeconfig, "kubeconfig", "", "path to management cluster kubeconfig") + + _ = cmd.MarkFlagRequired("from-file") + + return cmd +} + +func runUpdate(ctx context.Context, logger *log.Logger, name, fromFile, kubeconfigPath, kubeContext string) error { + data, err := os.ReadFile(fromFile) + if err != nil { + return fmt.Errorf("reading file %s: %w", fromFile, err) + } + + newObj := &unstructured.Unstructured{} + if err := yaml.Unmarshal(data, &newObj.Object); err != nil { + return fmt.Errorf("parsing YAML from %s: %w", fromFile, err) + } + + if kind := newObj.GetKind(); kind != "" && kind != "IdentityProvider" { + return fmt.Errorf("expected kind IdentityProvider, got %s", kind) + } + if fileName := newObj.GetName(); fileName != "" && fileName != name { + return fmt.Errorf("metadata.name %q in %s does not match NAME %q", fileName, fromFile, name) + } + + newSpec, found, err := unstructured.NestedMap(newObj.Object, "spec") + if err != nil { + return fmt.Errorf("reading spec from %s: %w", fromFile, err) + } + if !found || len(newSpec) == 0 { + return fmt.Errorf("no spec found in %s", fromFile) + } + + c, err := client.New(kubeconfigPath, kubeContext) + if err != nil { + return fmt.Errorf("connecting to management cluster: %w", err) + } + + // IdentityProvider is cluster-scoped: no namespace. + existing, err := c.Dynamic.Resource(client.IdentityProviderGVR).Get(ctx, name, metav1.GetOptions{}) + if err != nil { + return fmt.Errorf("getting IdentityProvider %s: %w", name, err) + } + + if err := mergeIdPSpec(existing, newSpec); err != nil { + return err + } + + _, err = c.Dynamic.Resource(client.IdentityProviderGVR).Update(ctx, existing, metav1.UpdateOptions{}) + if err != nil { + return fmt.Errorf("updating IdentityProvider: %w", err) + } + + logger.Success("identity provider updated", "name", name) + return nil +} + +// mergeIdPSpec applies newSpec onto existing, replacing only the spec. The +// existing status, resourceVersion, and the rest of metadata are left +// untouched so a spec update never clobbers the controller-managed status +// conditions and the Update carries the resourceVersion the API server +// requires for optimistic concurrency. existing is mutated in place. This +// mirrors the provider package's mergeProviderSpec discipline. +func mergeIdPSpec(existing *unstructured.Unstructured, newSpec map[string]interface{}) error { + if existing == nil || existing.Object == nil { + return fmt.Errorf("existing IdentityProvider is empty") + } + return unstructured.SetNestedMap(existing.Object, newSpec, "spec") +}