diff --git a/cmd/api_keys_test.go b/cmd/api_keys_test.go index ce1f3047..6632c6c7 100644 --- a/cmd/api_keys_test.go +++ b/cmd/api_keys_test.go @@ -157,6 +157,7 @@ func setupTestClient(t *testing.T) func() { origClient := ddClient origCfg := cfg origFactory := clientFactory + origAPIKeyFactory := apiKeyClientFactory // Create test config cfg = &config.Config{ @@ -166,10 +167,12 @@ func setupTestClient(t *testing.T) func() { AutoApprove: false, } - // Mock the client factory to return an error immediately - clientFactory = func(c *config.Config) (*client.Client, error) { + // Mock the client factories to return an error immediately + mockErr := func(c *config.Config) (*client.Client, error) { return nil, fmt.Errorf("mock client: no real API connection in tests") } + clientFactory = mockErr + apiKeyClientFactory = mockErr ddClient = nil @@ -178,6 +181,7 @@ func setupTestClient(t *testing.T) func() { ddClient = origClient cfg = origCfg clientFactory = origFactory + apiKeyClientFactory = origAPIKeyFactory } } diff --git a/cmd/notebooks.go b/cmd/notebooks.go index 8fa7529b..1818f27f 100644 --- a/cmd/notebooks.go +++ b/cmd/notebooks.go @@ -6,7 +6,11 @@ package cmd import ( + "encoding/json" "fmt" + "io" + "os" + "strings" "github.com/DataDog/datadog-api-client-go/v2/api/datadogV1" "github.com/DataDog/pup/pkg/formatter" @@ -35,8 +39,21 @@ EXAMPLES: # Get notebook details pup notebooks get notebook-id + # Create a notebook from file + pup notebooks create --body @notebook.json + + # Create from stdin + cat notebook.json | pup notebooks create --body - + + # Update a notebook + pup notebooks update 12345 --body @updated.json + + # Delete a notebook + pup notebooks delete 12345 + AUTHENTICATION: - Requires either OAuth2 authentication or API keys.`, + Requires API key authentication (DD_API_KEY + DD_APP_KEY). + OAuth2 is not supported for this endpoint.`, } var notebooksListCmd = &cobra.Command{ @@ -52,6 +69,19 @@ var notebooksGetCmd = &cobra.Command{ RunE: runNotebooksGet, } +var notebooksCreateCmd = &cobra.Command{ + Use: "create", + Short: "Create a new notebook", + RunE: runNotebooksCreate, +} + +var notebooksUpdateCmd = &cobra.Command{ + Use: "update [notebook-id]", + Short: "Update a notebook", + Args: cobra.ExactArgs(1), + RunE: runNotebooksUpdate, +} + var notebooksDeleteCmd = &cobra.Command{ Use: "delete [notebook-id]", Short: "Delete a notebook", @@ -60,11 +90,113 @@ var notebooksDeleteCmd = &cobra.Command{ } func init() { - notebooksCmd.AddCommand(notebooksListCmd, notebooksGetCmd, notebooksDeleteCmd) + notebooksCreateCmd.Flags().String("body", "", "JSON body (@filepath or - for stdin) (required)") + if err := notebooksCreateCmd.MarkFlagRequired("body"); err != nil { + panic(fmt.Errorf("failed to mark flag as required: %w", err)) + } + + notebooksUpdateCmd.Flags().String("body", "", "JSON body (@filepath or - for stdin) (required)") + if err := notebooksUpdateCmd.MarkFlagRequired("body"); err != nil { + panic(fmt.Errorf("failed to mark flag as required: %w", err)) + } + + notebooksCmd.AddCommand(notebooksListCmd, notebooksGetCmd, notebooksCreateCmd, notebooksUpdateCmd, notebooksDeleteCmd) +} + +// readBody reads JSON body content from a file (@path) or stdin (-). +func readBody(value string) ([]byte, error) { + var data []byte + var err error + + switch { + case value == "-": + data, err = io.ReadAll(inputReader) + if err != nil { + return nil, fmt.Errorf("failed to read body from stdin: %w", err) + } + case strings.HasPrefix(value, "@"): + path := strings.TrimPrefix(value, "@") + data, err = os.ReadFile(path) + if err != nil { + return nil, fmt.Errorf("failed to read body file: %w", err) + } + default: + return nil, fmt.Errorf("body must be @filepath or - for stdin") + } + + if !json.Valid(data) { + return nil, fmt.Errorf("invalid JSON in body") + } + + return data, nil +} + +func runNotebooksCreate(cmd *cobra.Command, args []string) error { + client, err := getClientForEndpoint("POST", "/api/v1/notebooks") + if err != nil { + return err + } + + bodyFlag, _ := cmd.Flags().GetString("body") + data, err := readBody(bodyFlag) + if err != nil { + return err + } + + var body datadogV1.NotebookCreateRequest + if err := json.Unmarshal(data, &body); err != nil { + return fmt.Errorf("failed to parse notebook create request: %w", err) + } + + api := datadogV1.NewNotebooksApi(client.V1()) + resp, r, err := api.CreateNotebook(client.Context(), body) + if err != nil { + return formatAPIError("create notebook", err, r) + } + + output, err := formatter.FormatOutput(resp, formatter.OutputFormat(outputFormat)) + if err != nil { + return err + } + printOutput("%s\n", output) + return nil +} + +func runNotebooksUpdate(cmd *cobra.Command, args []string) error { + client, err := getClientForEndpoint("PUT", "/api/v1/notebooks/") + if err != nil { + return err + } + + notebookID := parseInt64(args[0]) + + bodyFlag, _ := cmd.Flags().GetString("body") + data, err := readBody(bodyFlag) + if err != nil { + return err + } + + var body datadogV1.NotebookUpdateRequest + if err := json.Unmarshal(data, &body); err != nil { + return fmt.Errorf("failed to parse notebook update request: %w", err) + } + + api := datadogV1.NewNotebooksApi(client.V1()) + resp, r, err := api.UpdateNotebook(client.Context(), notebookID, body) + if err != nil { + return formatAPIError("update notebook", err, r) + } + + output, err := formatter.FormatOutput(resp, formatter.OutputFormat(outputFormat)) + if err != nil { + return err + } + printOutput("%s\n", output) + return nil } func runNotebooksList(cmd *cobra.Command, args []string) error { - client, err := getClient() + client, err := getClientForEndpoint("GET", "/api/v1/notebooks") if err != nil { return err } @@ -72,22 +204,19 @@ func runNotebooksList(cmd *cobra.Command, args []string) error { api := datadogV1.NewNotebooksApi(client.V1()) resp, r, err := api.ListNotebooks(client.Context()) if err != nil { - if r != nil { - return fmt.Errorf("failed to list notebooks: %w (status: %d)", err, r.StatusCode) - } - return fmt.Errorf("failed to list notebooks: %w", err) + return formatAPIError("list notebooks", err, r) } output, err := formatter.FormatOutput(resp, formatter.OutputFormat(outputFormat)) if err != nil { return err } - fmt.Println(output) + printOutput("%s\n", output) return nil } func runNotebooksGet(cmd *cobra.Command, args []string) error { - client, err := getClient() + client, err := getClientForEndpoint("GET", "/api/v1/notebooks/") if err != nil { return err } @@ -96,38 +225,35 @@ func runNotebooksGet(cmd *cobra.Command, args []string) error { api := datadogV1.NewNotebooksApi(client.V1()) resp, r, err := api.GetNotebook(client.Context(), notebookID) if err != nil { - if r != nil { - return fmt.Errorf("failed to get notebook: %w (status: %d)", err, r.StatusCode) - } - return fmt.Errorf("failed to get notebook: %w", err) + return formatAPIError("get notebook", err, r) } output, err := formatter.FormatOutput(resp, formatter.OutputFormat(outputFormat)) if err != nil { return err } - fmt.Println(output) + printOutput("%s\n", output) return nil } func runNotebooksDelete(cmd *cobra.Command, args []string) error { - client, err := getClient() + client, err := getClientForEndpoint("DELETE", "/api/v1/notebooks/") if err != nil { return err } notebookID := parseInt64(args[0]) if !cfg.AutoApprove { - fmt.Printf("⚠️ WARNING: This will permanently delete notebook %d\n", notebookID) - fmt.Print("Are you sure you want to continue? (y/N): ") - var response string - if _, err := fmt.Scanln(&response); err != nil { - // User cancelled or error reading input - fmt.Println("\nOperation cancelled") + printOutput("⚠️ WARNING: This will permanently delete notebook %d\n", notebookID) + printOutput("Are you sure you want to continue? (y/N): ") + + response, err := readConfirmation() + if err != nil { + printOutput("\nOperation cancelled\n") return nil } if response != "y" && response != "Y" { - fmt.Println("Operation cancelled") + printOutput("Operation cancelled\n") return nil } } @@ -135,12 +261,9 @@ func runNotebooksDelete(cmd *cobra.Command, args []string) error { api := datadogV1.NewNotebooksApi(client.V1()) r, err := api.DeleteNotebook(client.Context(), notebookID) if err != nil { - if r != nil { - return fmt.Errorf("failed to delete notebook: %w (status: %d)", err, r.StatusCode) - } - return fmt.Errorf("failed to delete notebook: %w", err) + return formatAPIError("delete notebook", err, r) } - fmt.Printf("Successfully deleted notebook %d\n", notebookID) + printOutput("Successfully deleted notebook %d\n", notebookID) return nil } diff --git a/cmd/notebooks_test.go b/cmd/notebooks_test.go index 9260f01c..5715073a 100644 --- a/cmd/notebooks_test.go +++ b/cmd/notebooks_test.go @@ -6,6 +6,10 @@ package cmd import ( + "bytes" + "os" + "path/filepath" + "strings" "testing" ) @@ -28,7 +32,7 @@ func TestNotebooksCmd(t *testing.T) { } func TestNotebooksCmd_Subcommands(t *testing.T) { - expectedCommands := []string{"list", "get", "delete"} + expectedCommands := []string{"list", "get", "create", "update", "delete"} commands := notebooksCmd.Commands() @@ -106,6 +110,153 @@ func TestNotebooksDeleteCmd(t *testing.T) { } } +func TestReadBody_File(t *testing.T) { + tmpFile := filepath.Join(t.TempDir(), "body.json") + content := []byte(`{"data":{"attributes":{"name":"test"}}}`) + if err := os.WriteFile(tmpFile, content, 0644); err != nil { + t.Fatal(err) + } + + got, err := readBody("@" + tmpFile) + if err != nil { + t.Fatalf("readBody returned error: %v", err) + } + if !bytes.Equal(got, content) { + t.Errorf("got %s, want %s", got, content) + } +} + +func TestReadBody_Stdin(t *testing.T) { + content := `{"data":{"attributes":{"name":"test"}}}` + origReader := inputReader + inputReader = strings.NewReader(content) + defer func() { inputReader = origReader }() + + got, err := readBody("-") + if err != nil { + t.Fatalf("readBody returned error: %v", err) + } + if string(got) != content { + t.Errorf("got %s, want %s", got, content) + } +} + +func TestReadBody_MissingFile(t *testing.T) { + _, err := readBody("@/nonexistent/path/body.json") + if err == nil { + t.Fatal("expected error for missing file") + } + if !strings.Contains(err.Error(), "failed to read body file") { + t.Errorf("error = %v, want 'failed to read body file'", err) + } +} + +func TestReadBody_InvalidJSON(t *testing.T) { + tmpFile := filepath.Join(t.TempDir(), "bad.json") + if err := os.WriteFile(tmpFile, []byte("not json"), 0644); err != nil { + t.Fatal(err) + } + + _, err := readBody("@" + tmpFile) + if err == nil { + t.Fatal("expected error for invalid JSON") + } + if !strings.Contains(err.Error(), "invalid JSON in body") { + t.Errorf("error = %v, want 'invalid JSON in body'", err) + } +} + +func TestReadBody_InvalidJSON_Stdin(t *testing.T) { + origReader := inputReader + inputReader = strings.NewReader("not json") + defer func() { inputReader = origReader }() + + _, err := readBody("-") + if err == nil { + t.Fatal("expected error for invalid JSON from stdin") + } + if !strings.Contains(err.Error(), "invalid JSON in body") { + t.Errorf("error = %v, want 'invalid JSON in body'", err) + } +} + +func TestReadBody_EmptyValue(t *testing.T) { + _, err := readBody("") + if err == nil { + t.Fatal("expected error for empty body value") + } +} + +func TestNotebooksCreateCmd(t *testing.T) { + if notebooksCreateCmd == nil { + t.Fatal("notebooksCreateCmd is nil") + } + + if notebooksCreateCmd.Use != "create" { + t.Errorf("Use = %s, want create", notebooksCreateCmd.Use) + } + + if notebooksCreateCmd.Short == "" { + t.Error("Short description is empty") + } + + if notebooksCreateCmd.RunE == nil { + t.Error("RunE is nil") + } + + flags := notebooksCreateCmd.Flags() + if flags.Lookup("body") == nil { + t.Error("Missing --body flag") + } +} + +func TestNotebooksCreateCmd_BodyRequired(t *testing.T) { + if notebooksCreateCmd.Flags().Lookup("body") == nil { + t.Fatal("--body flag not found") + } + + if err := notebooksCreateCmd.ValidateRequiredFlags(); err == nil { + t.Error("expected --body to be required") + } +} + +func TestNotebooksUpdateCmd_BodyRequired(t *testing.T) { + if notebooksUpdateCmd.Flags().Lookup("body") == nil { + t.Fatal("--body flag not found") + } + + if err := notebooksUpdateCmd.ValidateRequiredFlags(); err == nil { + t.Error("expected --body to be required") + } +} + +func TestNotebooksUpdateCmd(t *testing.T) { + if notebooksUpdateCmd == nil { + t.Fatal("notebooksUpdateCmd is nil") + } + + if notebooksUpdateCmd.Use != "update [notebook-id]" { + t.Errorf("Use = %s, want 'update [notebook-id]'", notebooksUpdateCmd.Use) + } + + if notebooksUpdateCmd.Short == "" { + t.Error("Short description is empty") + } + + if notebooksUpdateCmd.RunE == nil { + t.Error("RunE is nil") + } + + if notebooksUpdateCmd.Args == nil { + t.Error("Args validator is nil") + } + + flags := notebooksUpdateCmd.Flags() + if flags.Lookup("body") == nil { + t.Error("Missing --body flag") + } +} + func TestNotebooksCmd_ParentChild(t *testing.T) { commands := notebooksCmd.Commands() diff --git a/cmd/root.go b/cmd/root.go index f6ed1773..0eb5520f 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -28,6 +28,14 @@ func defaultClientFactory(cfg *config.Config) (*client.Client, error) { return client.New(cfg) } +// defaultAPIKeyClientFactory forces API key authentication +func defaultAPIKeyClientFactory(cfg *config.Config) (*client.Client, error) { + if err := cfg.Validate(); err != nil { + return nil, err + } + return client.NewWithAPIKeys(cfg) +} + var ( cfg *config.Config ddClient *client.Client @@ -36,6 +44,7 @@ var ( // Dependency injection points for testing clientFactory = defaultClientFactory + apiKeyClientFactory = defaultAPIKeyClientFactory outputWriter io.Writer = os.Stdout inputReader io.Reader = os.Stdin ) @@ -261,13 +270,8 @@ func getClientForEndpoint(method, path string) (*client.Client, error) { ) } - // Try to use the mocked factory if in test mode (allows test to fail intentionally) - // This respects the clientFactory mock in tests - c, err := clientFactory(cfg) - if err != nil { - return nil, err - } - return c, nil + // Force API key authentication for endpoints without OAuth support + return apiKeyClientFactory(cfg) } // Endpoint supports OAuth, use standard client diff --git a/pkg/client/auth_validator.go b/pkg/client/auth_validator.go index 45f9c089..cdc53c03 100644 --- a/pkg/client/auth_validator.go +++ b/pkg/client/auth_validator.go @@ -64,6 +64,13 @@ var endpointsWithoutOAuth = []EndpointAuthRequirement{ // Error Tracking API - OAuth not working in practice {Path: "/api/v2/error_tracking/issues/search", Method: "POST", SupportsOAuth: false, RequiresAPIKeys: true, Reason: "Error Tracking API requires API keys"}, {Path: "/api/v2/error_tracking/issues/", Method: "GET", SupportsOAuth: false, RequiresAPIKeys: true, Reason: "Error Tracking API requires API keys"}, + + // Notebooks API (V1) - missing OAuth scopes in spec + {Path: "/api/v1/notebooks", Method: "GET", SupportsOAuth: false, RequiresAPIKeys: true, Reason: "Notebooks API missing OAuth implementation in spec"}, + {Path: "/api/v1/notebooks", Method: "POST", SupportsOAuth: false, RequiresAPIKeys: true, Reason: "Notebooks API missing OAuth implementation in spec"}, + {Path: "/api/v1/notebooks/", Method: "GET", SupportsOAuth: false, RequiresAPIKeys: true, Reason: "Notebooks API missing OAuth implementation in spec"}, + {Path: "/api/v1/notebooks/", Method: "PUT", SupportsOAuth: false, RequiresAPIKeys: true, Reason: "Notebooks API missing OAuth implementation in spec"}, + {Path: "/api/v1/notebooks/", Method: "DELETE", SupportsOAuth: false, RequiresAPIKeys: true, Reason: "Notebooks API missing OAuth implementation in spec"}, } // AuthType represents the type of authentication being used