diff --git a/cmd/sync/prompt.go b/cmd/sync/prompt.go index 83d4e9783..70210b20a 100644 --- a/cmd/sync/prompt.go +++ b/cmd/sync/prompt.go @@ -6,6 +6,7 @@ import ( "strings" "github.com/spf13/cobra" + "github.com/spf13/pflag" "github.com/spf13/viper" "github.com/launchdarkly/ldcli/cmd/cliflags" @@ -20,11 +21,13 @@ import ( const ( archiveFlag = "archive" + configKeyFlag = "config-key" conflictFlag = "conflict" contentFlag = "content" diffFlag = "diff" dryRunFlag = "dry-run" formatFlag = "format" + keyFlag = "key" modelConfigKeyFlag = "model-config-key" nameFlag = "name" noInputFlag = "no-input" @@ -44,25 +47,21 @@ func NewPromptCmd(client resources.Client) *cobra.Command { ldcli sync prompts --dry-run # Add the first variation without prompting - ldcli sync prompts add production/support/default --no-input + ldcli sync prompts add --project production --config-key support --key default --no-input # Apply changes in an existing workspace without prompting ldcli sync prompts --yes --no-input - # Prefer local changes when conflicts occur - ldcli sync prompts --yes --conflict=local`, - Args: func(cmd *cobra.Command, args []string) error { - if err := cobra.NoArgs(cmd, args); err != nil { - return err - } - return validators.Validate()(cmd, args) - }, + # Sync one config, and prefer local changes when conflicts occur + ldcli sync prompts --project production --config-key support --yes --conflict=local`, + Args: validatedArgs(cobra.NoArgs), RunE: runPrompt(client, func(cmd *cobra.Command, _ []string) (syncprompt.CommandAction, error) { dryRun, _ := cmd.Flags().GetBool(dryRunFlag) - return syncprompt.SyncAction{DryRun: dryRun}, nil + return syncprompt.SyncAction{DryRun: dryRun, Filter: targetFilter(cmd)}, nil }), } + addTargetFlags(cmd, filterFlagHelp) addApplyFlags(cmd, true) cmd.AddCommand( newWatchCmd(client), @@ -129,10 +128,11 @@ func newWatchCmd(client resources.Client) *cobra.Command { # Apply watched changes without prompting and report unresolved conflicts ldcli sync prompts watch --yes --no-input`, Args: validatedArgs(cobra.NoArgs), - RunE: runPrompt(client, func(*cobra.Command, []string) (syncprompt.CommandAction, error) { - return syncprompt.SyncAction{Watch: true}, nil + RunE: runPrompt(client, func(cmd *cobra.Command, _ []string) (syncprompt.CommandAction, error) { + return syncprompt.SyncAction{Watch: true, Filter: targetFilter(cmd)}, nil }), } + addTargetFlags(cmd, filterFlagHelp) addApplyFlags(cmd, false) return cmd } @@ -144,11 +144,11 @@ func newAddCmd(client resources.Client) *cobra.Command { Example: ` # Choose variations interactively ldcli sync prompts add - # Add one variation by its full selector - ldcli sync prompts add production/support/default + # Choose variations in one config + ldcli sync prompts add --project production --config-key support - # Add multiple variations without prompting - ldcli sync prompts add production/support/default production/chat/concise --no-input`, + # Add variations by key without prompting + ldcli sync prompts add --project production --config-key support --key default --key concise --no-input`, Args: validatedArgs(cobra.MinimumNArgs(0)), RunE: runPrompt(client, func(cmd *cobra.Command, args []string) (syncprompt.CommandAction, error) { variations, err := parseVariationSelectors(args) @@ -156,9 +156,10 @@ func newAddCmd(client resources.Client) *cobra.Command { return nil, err } dryRun, _ := cmd.Flags().GetBool(dryRunFlag) - return syncprompt.AddAction{Variations: variations, DryRun: dryRun}, nil + return syncprompt.AddAction{Variations: variations, Filter: targetFilter(cmd), DryRun: dryRun}, nil }), } + addTargetFlags(cmd, questionFlagHelp) cmd.Flags().Bool(dryRunFlag, false, "Preview local files without creating them") cmd.Flags().Bool(diffFlag, false, "Show full diffs and file content in the plan") cmd.Flags().Bool(noInputFlag, false, "Fail instead of prompting for missing input") @@ -203,9 +204,10 @@ func newAttachKindCmd(client resources.Client, kind syncdomain.AttachmentKind) * if len(args) == 1 { key = args[0] } - return syncprompt.AttachAction{Kind: kind, Key: key, Target: target}, nil + return syncprompt.AttachAction{Kind: kind, Key: key, ProjectKey: targetFilter(cmd).Project, Target: target}, nil }), } + addTargetFlags(cmd, targetFlagHelp{project: questionFlagHelp.project}) cmd.Flags().String(toFlag, "", "Target variation (project-key/config-key/variation-key)") addApplyFlags(cmd, false) return cmd @@ -219,10 +221,10 @@ func newDetachCmd(client resources.Client) *cobra.Command { ldcli sync prompts detach # Stop syncing one variation - ldcli sync prompts detach production/support/default + ldcli sync prompts detach --project production --config-key support --key default # Stop syncing one variation and archive it in LaunchDarkly - ldcli sync prompts detach production/support/default --archive --yes --no-input`, + ldcli sync prompts detach --project production --config-key support --key default --archive --yes --no-input`, Args: validatedArgs(cobra.MinimumNArgs(0)), RunE: runPrompt(client, func(cmd *cobra.Command, args []string) (syncprompt.CommandAction, error) { variations, err := parseVariationSelectors(args) @@ -230,9 +232,10 @@ func newDetachCmd(client resources.Client) *cobra.Command { return nil, err } archive, _ := cmd.Flags().GetBool(archiveFlag) - return syncprompt.DetachAction{Variations: variations, Archive: archive}, nil + return syncprompt.DetachAction{Variations: variations, Filter: targetFilter(cmd), Archive: archive}, nil }), } + addTargetFlags(cmd, questionFlagHelp) cmd.Flags().Bool(archiveFlag, false, "Also archive the variations in LaunchDarkly") cmd.Flags().Bool(yesFlag, false, "Archive without confirmation") cmd.Flags().Bool(noInputFlag, false, "Fail instead of prompting for missing input") @@ -256,9 +259,10 @@ func newLinkCmd(client resources.Client) *cobra.Command { if err != nil { return nil, err } - return syncprompt.LinkAction{File: args[0], Format: format, Target: target}, nil + return syncprompt.LinkAction{File: args[0], Format: format, ProjectKey: targetFilter(cmd).Project, Target: target}, nil }), } + addTargetFlags(cmd, targetFlagHelp{project: questionFlagHelp.project}) cmd.Flags().String(formatFlag, "", "Format adapter (for example, plain-markdown)") cmd.Flags().String(toFlag, "", "New variation (project-key/config-key/variation-key)") cmd.Flags().String(modelConfigKeyFlag, "", "Model config key for the new variation") @@ -280,13 +284,83 @@ func addApplyFlags(cmd *cobra.Command, dryRun bool) { cmd.Flags().Bool(noInputFlag, false, "Fail instead of prompting for missing input") } +// targetFlagHelp is the help text of the target flags of one command. An +// empty text leaves out its flag. +type targetFlagHelp struct { + project string + configKey string + key string +} + +var ( + // filterFlagHelp describes target flags that limit a sync. + filterFlagHelp = targetFlagHelp{ + project: "Sync only this project", + configKey: "Sync only this config. Needs --project.", + key: "Sync only this variation. Repeat for more variations. Needs --config-key.", + } + // questionFlagHelp describes target flags that answer the questions of a command. + questionFlagHelp = targetFlagHelp{ + project: "Project key. Skips the project question.", + configKey: "Config key. Skips the config question. Needs --project.", + key: "Variation key. Repeat for more variations. Needs --config-key.", + } +) + +// addTargetFlags registers --project, --config-key, and --key. +func addTargetFlags(cmd *cobra.Command, help targetFlagHelp) { + cmd.Flags().String(cliflags.ProjectFlag, "", help.project) + if help.configKey != "" { + cmd.Flags().String(configKeyFlag, "", help.configKey) + } + if help.key != "" { + cmd.Flags().StringArray(keyFlag, nil, help.key) + } +} + +// targetFilter reads the target flags of a command. A flag that the command +// does not have is empty. +func targetFilter(cmd *cobra.Command) syncdomain.Filter { + project, _ := cmd.Flags().GetString(cliflags.ProjectFlag) + configKey, _ := cmd.Flags().GetString(configKeyFlag) + keys, _ := cmd.Flags().GetStringArray(keyFlag) + return syncdomain.Filter{Project: project, ConfigKey: configKey, Keys: keys} +} + +// validatedArgs checks the positional arguments and runs the shared ldcli +// validation. That validation also copies each LD_* environment variable and +// ldcli configuration value into the flag with the same name. Sync flags come +// only from the command line, so validatedArgs then resets each sync flag that +// the command line did not set. Global flags, such as --access-token, keep +// their environment and configuration values. func validatedArgs(validate cobra.PositionalArgs) cobra.PositionalArgs { return func(cmd *cobra.Command, args []string) error { if err := validate(cmd, args); err != nil { return err } - return validators.Validate()(cmd, args) + syncFlags := cmd.LocalNonPersistentFlags() + fromCommandLine := make(map[string]bool) + syncFlags.VisitAll(func(flag *pflag.Flag) { fromCommandLine[flag.Name] = flag.Changed }) + if err := validators.Validate()(cmd, args); err != nil { + return err + } + syncFlags.VisitAll(func(flag *pflag.Flag) { + if flag.Changed && !fromCommandLine[flag.Name] { + resetFlag(flag) + } + }) + return nil + } +} + +// resetFlag gives a flag its default value and marks it as not set. +func resetFlag(flag *pflag.Flag) { + if values, ok := flag.Value.(pflag.SliceValue); ok { + _ = values.Replace(nil) + } else { + _ = flag.Value.Set(flag.DefValue) } + flag.Changed = false } func parseVariationSelectors(values []string) ([]syncdomain.ResourceID, error) { diff --git a/cmd/sync/prompt_test.go b/cmd/sync/prompt_test.go index bf7c20086..1cc923aa0 100644 --- a/cmd/sync/prompt_test.go +++ b/cmd/sync/prompt_test.go @@ -1,11 +1,14 @@ package sync import ( + "strings" "testing" + "github.com/spf13/viper" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/launchdarkly/ldcli/cmd/cliflags" syncdomain "github.com/launchdarkly/ldcli/internal/sync" syncprompt "github.com/launchdarkly/ldcli/internal/sync/prompt" ) @@ -91,6 +94,75 @@ func TestPromptCommandsRegisterDiffFlag(t *testing.T) { } } +func TestPromptCommandsRegisterTargetFlags(t *testing.T) { + command := NewPromptCmd(nil) + tests := map[string]struct { + path []string + help targetFlagHelp + }{ + "prompts": {help: filterFlagHelp}, + "watch": {path: []string{"watch"}, help: filterFlagHelp}, + "add": {path: []string{"add"}, help: questionFlagHelp}, + "detach": {path: []string{"detach"}, help: questionFlagHelp}, + "attach tool": {path: []string{"attach", "tool"}, help: targetFlagHelp{project: questionFlagHelp.project}}, + "link": {path: []string{"link"}, help: targetFlagHelp{project: questionFlagHelp.project}}, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + target := command + if len(test.path) != 0 { + var err error + target, _, err = command.Find(test.path) + require.NoError(t, err) + } + + usage := func(name string) string { + if flag := target.Flags().Lookup(name); flag != nil { + return flag.Usage + } + return "" + } + assert.Equal(t, test.help, targetFlagHelp{ + project: usage(cliflags.ProjectFlag), configKey: usage(configKeyFlag), key: usage(keyFlag), + }) + assert.NotContains(t, target.Example, "production/support/default --", "examples use target flags") + }) + } +} + +// Sync flags come only from the command line. The shared ldcli validation +// copies environment variables and configuration values into every flag, so +// validatedArgs must undo that for the sync flags. +func TestValidatedArgsKeepsSyncFlagsFromTheCommandLineOnly(t *testing.T) { + t.Cleanup(viper.Reset) + viper.Reset() + viper.SetEnvPrefix("LD") + viper.SetEnvKeyReplacer(strings.NewReplacer("-", "_")) + viper.AutomaticEnv() + viper.SetConfigType("yaml") + require.NoError(t, viper.ReadConfig(strings.NewReader("project: from-config\nyes: true\noutput: json\n"))) + t.Setenv("LD_BASE_URI", "https://example.test") + t.Setenv("LD_CONFLICT", "bogus") + t.Setenv("LD_KEY", "from-environment") + t.Setenv("LD_NO_INPUT", "true") + t.Setenv("LD_RESOLVE", "demo/support/default=local") + + command := NewPromptCmd(nil) + require.NoError(t, command.ParseFlags([]string{"--dry-run", "--config-key", "support"})) + require.NoError(t, command.Args(command, nil)) + + for name, want := range map[string]string{ + cliflags.ProjectFlag: "", yesFlag: "false", conflictFlag: "", keyFlag: "[]", noInputFlag: "false", + resolveFlag: "[]", dryRunFlag: "true", configKeyFlag: "support", + } { + flag := command.Flags().Lookup(name) + assert.Equal(t, want, flag.Value.String(), name) + assert.Equal(t, want != flag.DefValue, flag.Changed, name) + } + assert.Equal(t, "https://example.test", viper.GetString(cliflags.BaseURIFlag), "global flags keep environment values") +} + func TestAttachSubcommandsAcceptOptionalKeyAndTarget(t *testing.T) { command := NewPromptCmd(nil) tool, _, err := command.Find([]string{"attach", "tool"}) diff --git a/internal/sync/bootstrap/bootstrap.go b/internal/sync/bootstrap/bootstrap.go index 86f1e23bf..15c766041 100644 --- a/internal/sync/bootstrap/bootstrap.go +++ b/internal/sync/bootstrap/bootstrap.go @@ -45,6 +45,9 @@ type Options struct { DryRun bool // Diff shows the content of each file that a dry run lists. Diff bool + // ProjectKey and ConfigKey skip the project and the config questions. + ProjectKey string + ConfigKey string // Selections are the variations to add. If it is empty, Run asks the user. Selections []syncdomain.ResourceID NoInput bool @@ -58,7 +61,7 @@ func Run(options Options) error { files, err = selectedVariations(options) } else { if err := syncinteractive.RequireTerminal( - options.Input, options.Output, options.NoInput, "variation selectors", "prompt selection", + options.Input, options.Output, options.NoInput, missingTargetFlags(options), "prompt selection", ); err != nil { return err } @@ -124,14 +127,37 @@ func selectedVariations(options Options) ([]synclocal.VariationFile, error) { return files, nil } +// missingTargetFlags names the flags that answer the questions that Run still +// has to ask. +func missingTargetFlags(options Options) string { + switch { + case options.ProjectKey == "": + return "--project, --config-key, and --key" + case options.ConfigKey == "": + return "--config-key and --key" + default: + return "--key values" + } +} + // promptForVariations asks the user for a project, a config, and one or more -// variations. The bool result is true when the user cancels. +// variations. ProjectKey and ConfigKey skip their questions. The bool result +// is true when the user cancels. func promptForVariations(options Options) ([]synclocal.VariationFile, bool, error) { - project, canceled, err := syncinteractive.SelectProject(options.Input, options.Output, options.Catalog) - if err != nil || canceled { - return nil, canceled, err + project := syncapi.Project{Key: options.ProjectKey} + var canceled bool + var err error + if project.Key == "" { + if project, canceled, err = syncinteractive.SelectProject(options.Input, options.Output, options.Catalog); err != nil || canceled { + return nil, canceled, err + } + } + config := syncapi.Config{Key: options.ConfigKey} + if config.Key == "" { + config, canceled, err = syncinteractive.SelectConfig(options.Input, options.Output, options.Catalog, project.Key, nil) + } else { + config, err = options.Catalog.Config(project.Key, config.Key) } - config, canceled, err := syncinteractive.SelectConfig(options.Input, options.Output, options.Catalog, project.Key, nil) if err != nil || canceled { return nil, canceled, err } diff --git a/internal/sync/bootstrap/bootstrap_test.go b/internal/sync/bootstrap/bootstrap_test.go index 55ebef9e9..b0d4b5aaa 100644 --- a/internal/sync/bootstrap/bootstrap_test.go +++ b/internal/sync/bootstrap/bootstrap_test.go @@ -87,6 +87,69 @@ func TestRunRequiresTerminal(t *testing.T) { ) } +func TestRunNamesTheMissingTargetFlagsWithoutInput(t *testing.T) { + tests := map[string]struct { + projectKey, configKey string + want string + }{ + "no flags": {want: "--project, --config-key, and --key are required with --no-input"}, + "project": {projectKey: "project", want: "--config-key and --key are required with --no-input"}, + "project and config": {projectKey: "project", configKey: "config", want: "--key values are required with --no-input"}, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + err := Run(Options{ + Catalog: &fakeCatalog{}, Store: synclocal.NewStore(t.TempDir()), + Input: bytes.NewBuffer(nil), Output: bytes.NewBuffer(nil), + ProjectKey: test.projectKey, ConfigKey: test.configKey, NoInput: true, + }) + + require.EqualError(t, err, test.want) + }) + } +} + +func TestPromptForVariationsSkipsTheQuestionsThatFlagsAnswer(t *testing.T) { + root := t.TempDir() + store := synclocal.NewStore(root) + variation := syncdomain.Variation{Key: "variation", Name: "Variation", Mode: syncdomain.VariationModeAgent, Instructions: "Be helpful."} + _, err := store.Add([]synclocal.VariationFile{{ProjectKey: "project", ConfigKey: "config", Variation: variation}}) + require.NoError(t, err) + catalog := &noSearchCatalog{config: syncapi.Config{ + Key: "config", Mode: syncdomain.VariationModeAgent, Variations: []syncdomain.Variation{variation}, + }} + + files, canceled, err := promptForVariations(Options{ + Catalog: catalog, Store: store, Input: bytes.NewBuffer(nil), Output: bytes.NewBuffer(nil), + ProjectKey: "project", ConfigKey: "config", + }) + + require.NoError(t, err) + assert.False(t, canceled) + assert.Empty(t, files, "every variation of the config is already synced") +} + +// noSearchCatalog fails when a question searches projects or configs. +type noSearchCatalog struct { + config syncapi.Config +} + +func (*noSearchCatalog) SearchProjects(string, int, int) (syncapi.Page[syncapi.Project], error) { + return syncapi.Page[syncapi.Project]{}, errors.New("asked for the project") +} + +func (*noSearchCatalog) SearchConfigs(string, string, []syncdomain.VariationMode, int, int) (syncapi.Page[syncapi.Config], error) { + return syncapi.Page[syncapi.Config]{}, errors.New("asked for the config") +} + +func (catalog *noSearchCatalog) Config(projectKey, configKey string) (syncapi.Config, error) { + if projectKey != "project" || configKey != catalog.config.Key { + return syncapi.Config{}, errors.New("read the wrong config") + } + return catalog.config, nil +} + func TestRunUsesExplicitVariationSelectorsWithoutTerminal(t *testing.T) { root := t.TempDir() var output bytes.Buffer diff --git a/internal/sync/detach/detach.go b/internal/sync/detach/detach.go index 764110510..bd7b92622 100644 --- a/internal/sync/detach/detach.go +++ b/internal/sync/detach/detach.go @@ -35,8 +35,10 @@ type Options struct { ProjectKeys []string Input io.Reader Output io.Writer - // Selections are the variations to detach. If it is empty, Run asks the user. + // Selections are the variations to detach. If it is empty, Run asks the + // user to choose from the synced resources that Filter keeps. Selections []syncdomain.ResourceID + Filter syncdomain.Filter NoInput bool // Archive also archives each selected variation in LaunchDarkly. Without // Yes, the user must confirm the archive. @@ -80,6 +82,7 @@ func Run(options Options) error { return err } } else { + synced = slices.DeleteFunc(synced, func(id syncdomain.ResourceID) bool { return !options.Filter.Includes(id) }) if len(synced) == 0 { _ = console.Line("No resources are currently synced.") return nil @@ -118,9 +121,11 @@ func Run(options Options) error { // promptForResources asks the user to choose one or more synced variations. // The bool result is true when the user cancels. func promptForResources(options Options, synced []syncdomain.ResourceID) ([]syncdomain.ResourceID, bool, error) { - if err := syncinteractive.RequireTerminal( - options.Input, options.Output, options.NoInput, "variation selectors", "resource selection", - ); err != nil { + arguments := "--project and --key" + if options.Filter.Project != "" { + arguments = "--key values" + } + if err := syncinteractive.RequireTerminal(options.Input, options.Output, options.NoInput, arguments, "resource selection"); err != nil { return nil, false, err } choices := make([]syncinteractive.Choice[syncdomain.ResourceID], 0, len(synced)) diff --git a/internal/sync/detach/detach_test.go b/internal/sync/detach/detach_test.go index c5e261859..c996406f7 100644 --- a/internal/sync/detach/detach_test.go +++ b/internal/sync/detach/detach_test.go @@ -227,6 +227,30 @@ func TestRunUsesExplicitSelectionsWithoutTerminal(t *testing.T) { assert.Equal(t, []string{"project"}, manifestStore.loadedProjectKeys) } +func TestRunListsOnlyTheResourcesThatTheFilterKeeps(t *testing.T) { + root := t.TempDir() + store := synclocal.NewStore(root) + _, err := store.Add([]synclocal.VariationFile{{ + ProjectKey: "project", ConfigKey: "config", Variation: testVariation("prompt"), + }}) + require.NoError(t, err) + options := Options{ + RepositoryRoot: root, Store: store, Baselines: newMemoryManifestStore(), + Input: bytes.NewBuffer(nil), NoInput: true, + } + + var otherProject bytes.Buffer + options.Output, options.Filter = &otherProject, syncdomain.Filter{Project: "other"} + require.NoError(t, Run(options)) + assert.Equal(t, "No resources are currently synced.\n", otherProject.String()) + + options.Output, options.Filter = bytes.NewBuffer(nil), syncdomain.Filter{Project: "project"} + require.EqualError(t, Run(options), "--key values are required with --no-input") + + options.Filter = syncdomain.Filter{} + require.EqualError(t, Run(options), "--project and --key are required with --no-input") +} + func TestRunReportsWhenNoResourcesAreSynced(t *testing.T) { root := t.TempDir() var output bytes.Buffer diff --git a/internal/sync/link/link.go b/internal/sync/link/link.go index 777999a2f..525d738ac 100644 --- a/internal/sync/link/link.go +++ b/internal/sync/link/link.go @@ -43,6 +43,8 @@ type Options struct { Format string Input io.Reader Output io.Writer + // ProjectKey skips the project question when Run asks for the destination. + ProjectKey string // Target is the new variation. If it is nil, Run asks the user. Target *Target NoInput bool @@ -163,15 +165,26 @@ func targetSelection(options Options, prompt linkedPrompt, target Target) (Selec // that the linked file does not have. The bool result is true when the user // cancels. func promptForSelection(options Options, prompt linkedPrompt) (Selection, string, bool, error) { - project, canceled, err := syncinteractive.SelectProject(options.Input, options.Output, options.Catalog) - if err != nil || canceled { - return Selection{}, "", canceled, err + project := syncapi.Project{Key: options.ProjectKey} + if project.Key == "" { + var canceled bool + var err error + if project, canceled, err = syncinteractive.SelectProject(options.Input, options.Output, options.Catalog); err != nil || canceled { + return Selection{}, "", canceled, err + } } var modes []syncdomain.VariationMode if prompt.parsed.Mode != "" { modes = []syncdomain.VariationMode{prompt.parsed.Mode} } config, canceled, err := syncinteractive.SelectConfig(options.Input, options.Output, options.Catalog, project.Key, modes) + if syncapi.IsNotFound(err) && options.ProjectKey != "" { + // The config search is the first read in the project, so a 404 means + // that the project is missing. + return Selection{}, "", false, fmt.Errorf( + "project %q was not found in LaunchDarkly, or the access token cannot read it", options.ProjectKey, + ) + } if err != nil || canceled { return Selection{}, "", canceled, err } diff --git a/internal/sync/link/link_test.go b/internal/sync/link/link_test.go index 675703e69..906e55bd8 100644 --- a/internal/sync/link/link_test.go +++ b/internal/sync/link/link_test.go @@ -1,6 +1,8 @@ package link import ( + "bytes" + "errors" "os" "path/filepath" "testing" @@ -218,6 +220,35 @@ func TestCreateDoesNotOverwriteLinkedFileChangedAfterRead(t *testing.T) { require.Equal(t, "newer content from the editor", string(content)) } +func TestPromptForSelectionSkipsTheProjectQuestionAndNamesAMissingProject(t *testing.T) { + catalog := &missingProjectCatalog{} + + _, _, _, err := promptForSelection(Options{ + Catalog: catalog, Input: bytes.NewBuffer(nil), Output: bytes.NewBuffer(nil), ProjectKey: "demo", + }, linkedPrompt{}) + + require.EqualError(t, err, `project "demo" was not found in LaunchDarkly, or the access token cannot read it`) + require.Equal(t, []string{"demo"}, catalog.configSearches) +} + +// missingProjectCatalog fails a project search, and answers a config search +// with a 404 as LaunchDarkly does for a project that does not exist. +type missingProjectCatalog struct { + fakeCatalog + configSearches []string +} + +func (*missingProjectCatalog) SearchProjects(string, int, int) (syncapi.Page[syncapi.Project], error) { + return syncapi.Page[syncapi.Project]{}, errors.New("asked for the project") +} + +func (catalog *missingProjectCatalog) SearchConfigs( + projectKey, _ string, _ []syncdomain.VariationMode, _, _ int, +) (syncapi.Page[syncapi.Config], error) { + catalog.configSearches = append(catalog.configSearches, projectKey) + return syncapi.Page[syncapi.Config]{}, errors.New(`{"code":"not_found","statusCode":404}`) +} + func TestRequiredValueRejectsBlankInput(t *testing.T) { require.Error(t, requiredValue("variation name")(" ")) require.NoError(t, requiredValue("variation name")("Custom name")) diff --git a/internal/sync/manifest/baseline.go b/internal/sync/manifest/baseline.go index 1b8002843..3f3b69618 100644 --- a/internal/sync/manifest/baseline.go +++ b/internal/sync/manifest/baseline.go @@ -1,8 +1,6 @@ package manifest import ( - "slices" - syncdomain "github.com/launchdarkly/ldcli/internal/sync" ) @@ -54,16 +52,19 @@ func NewBaselineStore(remote Store, lock LockFile) BaselineStore { return BaselineStore{remote: remote, lock: lock} } -// Load reads the sync.lock file and the remote manifest of each project that -// projectKeys or the lock names. A working copy without a sync.lock file -// tracks nothing yet. The remote manifest is not its lock, because other -// branches add entries to it that this working copy never had. +// Load reads the sync.lock file and the remote manifest of each project in +// projectKeys. A working copy without a sync.lock file tracks nothing yet. The +// remote manifest is not its lock, because other branches add entries to it +// that this working copy never had. +// +// Save sends only the entries that changed, so a project that Load did not +// read keeps its remote manifest as long as its lock entries do not change. func (store BaselineStore) Load(projectKeys []string) (Baseline, error) { lock, err := ReadLock(store.lock) if err != nil { return Baseline{}, err } - remote, err := store.remote.Load(append(slices.Clone(projectKeys), lock.ProjectKeys()...)) + remote, err := store.remote.Load(projectKeys) if err != nil { return Baseline{}, err } diff --git a/internal/sync/manifest/baseline_test.go b/internal/sync/manifest/baseline_test.go index 2acb590e7..84afcc53c 100644 --- a/internal/sync/manifest/baseline_test.go +++ b/internal/sync/manifest/baseline_test.go @@ -82,7 +82,7 @@ func TestBaselineStoreUsesLockFileAsBaselineAndReportsStaleness(t *testing.T) { client := &manifestClient{manifests: map[string]syncapi.SyncManifest{"production": remoteManifest(fingerprint("b"), 5)}} lock := lockWith(t, fingerprint("a"), 4) - baseline, err := NewBaselineStore(NewStore(client, testSource), lock).Load(nil) + baseline, err := NewBaselineStore(NewStore(client, testSource), lock).Load([]string{"production"}) require.NoError(t, err) assert.Equal(t, fingerprint("a"), baseline.Lock.Resources[0].Fingerprint) @@ -103,7 +103,7 @@ func TestBaselineStaleComparesFingerprints(t *testing.T) { for name, test := range tests { t.Run(name, func(t *testing.T) { client := &manifestClient{manifests: map[string]syncapi.SyncManifest{"production": test.remote}} - baseline, err := NewBaselineStore(NewStore(client, testSource), lockWith(t, fingerprint("a"), 4)).Load(nil) + baseline, err := NewBaselineStore(NewStore(client, testSource), lockWith(t, fingerprint("a"), 4)).Load([]string{"production"}) require.NoError(t, err) assert.Equal(t, test.stale, baseline.Stale(variationID())) @@ -126,7 +126,7 @@ func TestBaselineStoreSaveSendsRemoteVersionsAndWritesLockWithoutThem(t *testing } lock := lockWith(t, fingerprint("a"), 4) store := NewBaselineStore(NewStore(client, testSource), lock) - baseline, err := store.Load(nil) + baseline, err := store.Load([]string{"production"}) require.NoError(t, err) next := baseline.Lock.Clone() @@ -151,7 +151,7 @@ func TestBaselineStoreSaveKeepsRemoteEntryThatThisCopyDidNotChange(t *testing.T) client := &manifestClient{manifests: map[string]syncapi.SyncManifest{"production": remoteManifest(fingerprint("b"), 5)}} lock := lockWith(t, fingerprint("a"), 4) store := NewBaselineStore(NewStore(client, testSource), lock) - baseline, err := store.Load(nil) + baseline, err := store.Load([]string{"production"}) require.NoError(t, err) _, err = store.Save(baseline, baseline.Lock) @@ -168,7 +168,7 @@ func TestBaselineStoreSaveDoesNotWriteLockWhenRemoteSaveFails(t *testing.T) { lock := lockWith(t, fingerprint("a"), 4) before := string(lock.content) store := NewBaselineStore(NewStore(client, testSource), lock) - baseline, err := store.Load(nil) + baseline, err := store.Load([]string{"production"}) require.NoError(t, err) next := baseline.Lock.Clone() @@ -186,7 +186,7 @@ func TestBaselineStoreSaveRemovesEmptyLock(t *testing.T) { } lock := lockWith(t, fingerprint("a"), 4) store := NewBaselineStore(NewStore(client, testSource), lock) - baseline, err := store.Load(nil) + baseline, err := store.Load([]string{"production"}) require.NoError(t, err) _, err = store.Save(baseline, New()) @@ -198,6 +198,49 @@ func TestBaselineStoreSaveRemovesEmptyLock(t *testing.T) { }}, client.patches[0].deletions) } +// A filtered sync loads only its own project. The lock entries of every other +// project must stay as they are, and their remote manifests are not read. +func TestBaselineStoreLoadsOnlyTheNamedProjects(t *testing.T) { + client := &readRecordingClient{manifestClient: &manifestClient{ + manifests: map[string]syncapi.SyncManifest{"production": remoteManifest(fingerprint("a"), 4)}, + patchResponses: []syncapi.SyncManifest{remoteManifest(fingerprint("c"), 5)}, + }} + other := Resource{ + ResourceKind: syncdomain.KindVariation, ProjectKey: "other", LookupKey: "chat/default", Fingerprint: fingerprint("e"), + } + content, err := encodeLock(Manifest{Resources: []Resource{ + {ResourceKind: syncdomain.KindVariation, ProjectKey: "production", LookupKey: "support/default", Fingerprint: fingerprint("a")}, + other, + }}) + require.NoError(t, err) + lock := &memoryLock{content: content} + store := NewBaselineStore(NewStore(client, testSource), lock) + + baseline, err := store.Load([]string{"production"}) + require.NoError(t, err) + next := baseline.Lock.Clone() + next.SetFingerprint(variationID(), fingerprint("c")) + _, err = store.Save(baseline, next) + + require.NoError(t, err) + assert.Equal(t, []string{"production"}, client.reads) + require.Len(t, client.patches, 1) + assert.Equal(t, "production", client.patches[0].projectKey) + written, err := ReadLock(lock) + require.NoError(t, err) + assert.Contains(t, written.Resources, other) +} + +type readRecordingClient struct { + *manifestClient + reads []string +} + +func (client *readRecordingClient) GetSyncManifest(projectKey, source string) (syncapi.SyncManifest, error) { + client.reads = append(client.reads, projectKey) + return client.manifestClient.GetSyncManifest(projectKey, source) +} + func variationID() syncdomain.ResourceID { return syncdomain.VariationID("production", "support", "default") } diff --git a/internal/sync/prompt/acceptance_test.go b/internal/sync/prompt/acceptance_test.go index 53736e44d..44a92ecc5 100644 --- a/internal/sync/prompt/acceptance_test.go +++ b/internal/sync/prompt/acceptance_test.go @@ -911,6 +911,218 @@ func deleteVariationFile(t *testing.T, root string) { require.NoError(t, err) } +func TestPromptFilterSyncsOnlyTheNamedVariation(t *testing.T) { + root := initRepository(t) + baseline := variation("Baseline") + writeVariation(t, root, variation("Local"), true) + writeManifest(t, root, baseline) + other := addOtherProjectVariation(t, root) + lockBefore := readLock(t, root) + api := &directAPI{variation: pointer(baseline)} + + _, _, err := runPrompt(t, root, api, "--project", "production", "--config-key", "support", "--key", "default", "--yes") + + require.NoError(t, err) + assert.Equal(t, "Local", api.variation.Name) + for _, request := range api.requests { + assert.NotContains(t, request, "/projects/other/") + } + lockAfter := readLock(t, root) + assert.Contains(t, lockAfter.Resources, other) + assert.Contains(t, lockBefore.Resources, other) + assert.NotEqual(t, lockBefore, lockAfter) +} + +func TestPromptTargetFlagErrorsComeBeforeAnyRequest(t *testing.T) { + tests := map[string]struct { + arguments []string + want string + }{ + "key without project": {[]string{"--key", "default"}, "--key needs --project"}, + "key without config": { + []string{"--project", "production", "--key", "default"}, + "--key needs --config-key. In sync prompts, --key is a variation key.", + }, + "config without project": {[]string{"--config-key", "support"}, "--config-key needs --project"}, + "blank project": {[]string{"--project", " "}, "--project cannot be blank"}, + "repeated key": { + []string{"--project", "production", "--config-key", "support", "--key", "default", "--key", "default"}, + "variation production/support/default was selected more than once", + }, + "key that is not synced": { + []string{"--project", "production", "--config-key", "support", "--key", "serch"}, + "variation production/support/serch is not synced in this repository. " + + "To start syncing it, run ldcli sync prompts add --project production --config-key support --key serch.", + }, + "project that is not synced": { + []string{"--project", "demo"}, + `project "demo" has no synced resources in this repository. To start, run ldcli sync prompts add --project demo.`, + }, + "config that is not synced": { + []string{"--project", "production", "--config-key", "chat"}, + `config "chat" in project "production" has no synced variations in this repository. ` + + "To start, run ldcli sync prompts add --project production --config-key chat.", + }, + "selectors and target flags": { + []string{"add", "production/support/other", "--project", "production"}, + "pass variations as --project, --config-key, and --key, or as project-key/config-key/variation-key arguments, not both", + }, + "project that --to does not name": { + []string{"attach", "tool", "search", "--project", "production", "--to", "other/support/default"}, + `--to "other/support/default" names project "other", but --project is "production"`, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + root := initRepository(t) + writeVariation(t, root, variation("Baseline"), true) + writeManifest(t, root, variation("Baseline")) + api := &directAPI{} + + _, _, err := runPrompt(t, root, api, append(test.arguments, "--no-input")...) + + require.ErrorContains(t, err, test.want) + assert.Empty(t, api.requests) + }) + } +} + +func TestPromptAddsVariationsNamedByTargetFlags(t *testing.T) { + root := initRepository(t) + api := &directAPI{variation: pointer(variation("Remote"))} + + stdout, _, err := runPrompt(t, root, api, "add", "--project", "production", "--config-key", "support", "--key", "default", "--no-input") + + require.NoError(t, err) + assert.Contains(t, stdout, "Bootstrapped 1 variation file") + assertManifestFingerprint(t, root, variation("Remote")) +} + +// Sync flags come only from the command line. A user's environment must not +// apply, filter, or resolve a sync. +func TestPromptIgnoresSyncFlagsFromTheEnvironment(t *testing.T) { + root := initRepository(t) + writeVariation(t, root, variation("Local"), true) + writeManifest(t, root, variation("Baseline")) + api := &directAPI{variation: pointer(variation("Baseline"))} + t.Setenv("LD_YES", "true") + t.Setenv("LD_CONFLICT", "bogus") + t.Setenv("LD_PROJECT", "demo") + t.Setenv("LD_KEY", "default") + t.Setenv("LD_NO_INPUT", "true") + + _, _, err := runPrompt(t, root, api) + require.ErrorContains(t, err, "interactive confirmation requires a terminal") + requireOnlyReads(t, api.requests) + + _, _, err = runPrompt(t, root, api, "add") + require.ErrorContains(t, err, "interactive prompt selection requires a terminal") +} + +func TestPromptReadsTheAccessTokenFromTheEnvironment(t *testing.T) { + root := initRepository(t) + writeVariation(t, root, variation("Baseline"), true) + writeManifest(t, root, variation("Baseline")) + api := &directAPI{variation: pointer(variation("Baseline")), manifest: manifestsByRoot[root]} + t.Chdir(root) + t.Setenv("XDG_CONFIG_HOME", t.TempDir()) + t.Setenv("LD_ACCESS_TOKEN", "token") + + stdout, _, err := cmd.CallCmdCapturingStderr(t, cmd.APIClients{ResourcesClient: api}, analytics.NoopClientFn{}.Tracker(), + []string{"sync", "prompts", "--base-uri", "https://example.test", "--dry-run", "--output", "plaintext"}) + + require.NoError(t, err) + assert.Equal(t, "No changes. 1 in sync.\n", string(stdout)) +} + +// A filtered sync does not see the variations outside the filter. A missing +// file of such a variation must not make its tool look unused, because a full +// sync restores the file. +func TestPromptFilterKeepsToolsOfVariationsOutsideTheFilter(t *testing.T) { + tests := map[string]struct { + local string + }{ + "with a change": {local: "Local"}, + "without a change": {local: "Baseline"}, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + root := initRepository(t) + baseline := variation("Baseline") + writeVariation(t, root, variation(test.local), true) + writeManifest(t, root, baseline) + toolID := addOtherProjectVariationWithTool(t, root) + toolPath := filepath.Join(root, syncdomain.RootDir, "other", "tools", "search.json") + api := &directAPI{variation: pointer(baseline)} + + _, _, err := runPrompt(t, root, api, "--project", "production", "--yes") + + require.NoError(t, err) + assert.Equal(t, test.local, api.variation.Name) + assert.FileExists(t, toolPath) + assert.True(t, slices.ContainsFunc(readLock(t, root).Resources, func(resource syncmanifest.Resource) bool { + return resource.ID() == toolID + }), "the sync.lock file must keep the tool of the other project") + for _, request := range api.requests { + assert.NotContains(t, request, "/projects/other/") + } + }) + } +} + +// addOtherProjectVariationWithTool tracks a variation that uses the tool +// "search" in a second project. The tool file exists, but the variation file +// is missing, as after a branch switch. It returns the ID of the tool. +func addOtherProjectVariationWithTool(t *testing.T, root string) syncdomain.ResourceID { + t.Helper() + store := synclocal.NewStore(root) + value := variation("Other") + value.Tools = []syncdomain.AttachmentRef{{Key: "search", Version: 1}} + value.Attachments = []syncdomain.Attachment{toolAttachment(syncdomain.Tool{Key: "search", Schema: map[string]any{"type": "object"}}, 1)} + _, err := store.Add([]synclocal.VariationFile{{ProjectKey: "other", ConfigKey: "chat", Upsert: true, Variation: value}}) + require.NoError(t, err) + + lock := readLock(t, root) + require.NoError(t, lock.SetAttachments("other", value.Attachments)) + id := syncdomain.VariationID("other", "chat", "default") + fingerprint, err := syncdomain.FingerprintVariation(id.ProjectKey, id.LookupKey, value) + require.NoError(t, err) + lock.SetFingerprint(id, fingerprint) + require.NoError(t, syncmanifest.WriteLock(store, lock)) + + _, err = store.DeleteVariations([]synclocal.VariationDeletion{{ProjectKey: "other", ConfigKey: "chat", VariationKey: "default"}}) + require.NoError(t, err) + require.FileExists(t, filepath.Join(root, syncdomain.RootDir, "other", "tools", "search.json")) + return value.Attachments[0].ID("other") +} + +// addOtherProjectVariation syncs a variation in a second project and returns +// its sync.lock entry. +func addOtherProjectVariation(t *testing.T, root string) syncmanifest.Resource { + t.Helper() + store := synclocal.NewStore(root) + value := variation("Other") + _, err := store.Add([]synclocal.VariationFile{{ProjectKey: "other", ConfigKey: "chat", Upsert: true, Variation: value}}) + require.NoError(t, err) + id := syncdomain.VariationID("other", "chat", "default") + fingerprint, err := syncdomain.FingerprintVariation(id.ProjectKey, id.LookupKey, value) + require.NoError(t, err) + + lock := readLock(t, root) + lock.SetFingerprint(id, fingerprint) + require.NoError(t, syncmanifest.WriteLock(store, lock)) + return syncmanifest.Resource{ResourceKind: id.Kind, ProjectKey: id.ProjectKey, LookupKey: id.LookupKey, Fingerprint: fingerprint} +} + +func readLock(t *testing.T, root string) syncmanifest.Manifest { + t.Helper() + lock, err := syncmanifest.ReadLock(synclocal.NewStore(root)) + require.NoError(t, err) + return lock +} + func TestPromptRestoresTrackedMissingFileInsteadOfArchiving(t *testing.T) { root := initRepository(t) baseline := variation("Baseline") diff --git a/internal/sync/prompt/action.go b/internal/sync/prompt/action.go index da3c0f2f2..eafc6d21e 100644 --- a/internal/sync/prompt/action.go +++ b/internal/sync/prompt/action.go @@ -3,6 +3,7 @@ package prompt import ( "context" "errors" + "fmt" "io" syncdomain "github.com/launchdarkly/ldcli/internal/sync" @@ -15,48 +16,64 @@ type CommandAction interface { validate() error } -// SyncAction reconciles the workspace once, or each time a watched file changes. +// SyncAction reconciles the workspace once, or each time a watched file +// changes. Filter limits the sync to the resources that it names. type SyncAction struct { Watch bool DryRun bool + Filter syncdomain.Filter } -// AddAction writes selected LaunchDarkly variations as local files. +// AddAction writes selected LaunchDarkly variations as local files. The +// variations come from positional selectors or from the target flags in +// Filter, which also skip the project and config questions. type AddAction struct { Variations []syncdomain.ResourceID + Filter syncdomain.Filter DryRun bool } // AttachAction adds a tool or a skill to a local variation, and then syncs. +// ProjectKey skips the project question. type AttachAction struct { - Kind syncdomain.AttachmentKind - Key string - Target *syncdomain.ResourceID + Kind syncdomain.AttachmentKind + Key string + ProjectKey string + Target *syncdomain.ResourceID } // DetachAction stops syncing selected local variations. With Archive, it -// also archives them in LaunchDarkly. +// also archives them in LaunchDarkly. The variations come from positional +// selectors or from the target flags in Filter. type DetachAction struct { Variations []syncdomain.ResourceID + Filter syncdomain.Filter Archive bool } -// LinkAction creates a variation whose prompt is an external file, and then syncs. +// LinkAction creates a variation whose prompt is an external file, and then +// syncs. ProjectKey skips the project question. type LinkAction struct { - File string - Format string - Target *synclink.Target + File string + Format string + ProjectKey string + Target *synclink.Target } func (action SyncAction) validate() error { if action.Watch && action.DryRun { return errors.New("watch does not support --dry-run") } - return nil + return action.Filter.Validate(syncdomain.KindVariation) +} + +func (action AddAction) validate() error { + return validateSelections(action.Variations, action.Filter) } -func (AddAction) validate() error { return nil } -func (DetachAction) validate() error { return nil } +func (action DetachAction) validate() error { + return validateSelections(action.Variations, action.Filter) +} func (action AttachAction) validate() error { if !action.Kind.Valid() { @@ -65,7 +82,7 @@ func (action AttachAction) validate() error { if action.Target != nil && action.Target.Kind != syncdomain.KindVariation { return errors.New("attachment target must be a variation") } - return nil + return validateProjectFlag(action.ProjectKey, action.Target) } func (action LinkAction) validate() error { @@ -77,9 +94,49 @@ func (action LinkAction) validate() error { case action.Target != nil && action.Target.ModelConfigKey == "": return errors.New("--model-config-key is required with --to") } + var target *syncdomain.ResourceID + if action.Target != nil { + target = &action.Target.Variation + } + if err := validateProjectFlag(action.ProjectKey, target); err != nil { + return err + } return syncreference.ValidateFormat(action.Format) } +// validateSelections rejects variations that come both from positional +// selectors and from target flags. +func validateSelections(selectors []syncdomain.ResourceID, filter syncdomain.Filter) error { + if err := filter.Validate(syncdomain.KindVariation); err != nil { + return err + } + if len(selectors) != 0 && !filter.IsZero() { + return errors.New("pass variations as --project, --config-key, and --key, " + + "or as project-key/config-key/variation-key arguments, not both") + } + return nil +} + +// validateProjectFlag rejects an unsafe --project, or one that names another +// project than --to. +func validateProjectFlag(projectKey string, target *syncdomain.ResourceID) error { + if err := (syncdomain.Filter{Project: projectKey}).Validate(syncdomain.KindVariation); err != nil { + return err + } + if projectKey != "" && target != nil && target.ProjectKey != projectKey { + return fmt.Errorf("--to %q names project %q, but --project is %q", target.String(), target.ProjectKey, projectKey) + } + return nil +} + +// selections returns the variations that positional selectors or --key name. +func selections(selectors []syncdomain.ResourceID, filter syncdomain.Filter) []syncdomain.ResourceID { + if len(selectors) != 0 { + return selectors + } + return filter.IDs(syncdomain.KindVariation) +} + // Options are the command input and the streams of one prompt sync command. type Options struct { WorkingDirectory string @@ -138,3 +195,10 @@ func (options Options) watching() bool { action, ok := options.Action.(SyncAction) return ok && action.Watch } + +// syncAction returns the sync that the command runs. After an attach or a +// link, it is a sync of every resource. +func (options Options) syncAction() SyncAction { + action, _ := options.Action.(SyncAction) + return action +} diff --git a/internal/sync/prompt/execute.go b/internal/sync/prompt/execute.go index 645fda533..94cd4ddc3 100644 --- a/internal/sync/prompt/execute.go +++ b/internal/sync/prompt/execute.go @@ -13,7 +13,8 @@ import ( // executePlan applies each action of the plan, and returns the outcomes and // the new manifest. The manifest records only the actions that succeed. The // manifest argument does not change. localFiles has each local variation as -// its file stores it. +// its file stores it. filter is the filter of the plan. A filtered plan keeps +// every tool and skill entry, because it does not see the other variations. func executePlan( repositoryRoot string, localStore synclocal.Store, @@ -21,6 +22,7 @@ func executePlan( manifest syncmanifest.Manifest, plan Plan, localFiles map[ResourceID]syncdomain.SyncedResource, + filter syncdomain.Filter, ) ([]ResourceOutcome, syncmanifest.Manifest, error) { // A conflict choice applies to the full plan. Write nothing until every // conflict has a choice, so that a shared attachment cannot change while @@ -75,7 +77,9 @@ func executePlan( if err != nil { failures = append(failures, err) } else { - next.RemoveUnusedAttachments(variations) + if filter.IsZero() { + next.RemoveUnusedAttachments(variations) + } next.SetRefs(variations) } } diff --git a/internal/sync/prompt/execute_test.go b/internal/sync/prompt/execute_test.go index dc6667354..d4f757281 100644 --- a/internal/sync/prompt/execute_test.go +++ b/internal/sync/prompt/execute_test.go @@ -31,7 +31,7 @@ func TestExecutePlanDoesNotMutateReviewedManifest(t *testing.T) { LocalFingerprint: "updated", }}} - _, updatedManifest, err := executePlan("", synclocal.Store{}, syncapi.Client{}, reviewedManifest, plan, nil) + _, updatedManifest, err := executePlan("", synclocal.Store{}, syncapi.Client{}, reviewedManifest, plan, nil, syncdomain.Filter{}) require.NoError(t, err) assert.Equal(t, "reviewed", reviewedManifest.Resources[0].Fingerprint) @@ -77,7 +77,7 @@ func TestExecutePlanDoesNotMutateWhileConflictIsUnresolved(t *testing.T) { }, }} - outcomes, _, err := executePlan("", synclocal.Store{}, client, syncmanifest.New(), plan, nil) + outcomes, _, err := executePlan("", synclocal.Store{}, client, syncmanifest.New(), plan, nil, syncdomain.Filter{}) require.ErrorContains(t, err, "cannot sync conflicted resource") assert.Empty(t, outcomes) @@ -119,7 +119,7 @@ func TestExecutePlanSkipsEveryConsumerWhenSharedAttachmentFails(t *testing.T) { syncdomain.ResourceID{Kind: syncdomain.KindTool, ProjectKey: "project", LookupKey: "search"}, "sha256:reviewed", ) - outcomes, manifest, err := executePlan("", synclocal.Store{}, client, originalManifest, plan, nil) + outcomes, manifest, err := executePlan("", synclocal.Store{}, client, originalManifest, plan, nil, syncdomain.Filter{}) require.Error(t, err) require.Len(t, outcomes, 2) diff --git a/internal/sync/prompt/review.go b/internal/sync/prompt/review.go index 9e4771415..64ad9d04c 100644 --- a/internal/sync/prompt/review.go +++ b/internal/sync/prompt/review.go @@ -55,8 +55,13 @@ func reviewAndConfirmPlan(options Options, plan Plan, interactive bool) (bool, e // cleanupOrphanedAttachments shows the tool and skill files that no local // variation uses, and deletes them after the user agrees. Without --yes and // without a terminal, it keeps the files. Before it deletes a file, it makes -// sure again that no variation uses the file. +// sure again that no variation uses the file. A filtered sync keeps every +// file. A variation outside the filter can have a missing file that a full +// sync restores, and then its tools and skills only look unused. func cleanupOrphanedAttachments(options Options, store synclocal.Store, interactive bool) error { + if !options.syncAction().Filter.IsZero() { + return nil + } orphaned, err := store.OrphanedAttachments() if err != nil || len(orphaned) == 0 { return err diff --git a/internal/sync/prompt/runner.go b/internal/sync/prompt/runner.go index 80e8a1d5d..e6917df12 100644 --- a/internal/sync/prompt/runner.go +++ b/internal/sync/prompt/runner.go @@ -100,18 +100,20 @@ func (runner Runner) Run(options Options) error { } // runSync syncs once, or watches the workspace. A workspace without any sync -// files starts with an add, so that the user can choose the first variations. +// files starts with an add in the filter project, so that the user can choose +// the first variations. func (runner Runner) runSync(options Options, workspace syncWorkspace, action SyncAction) error { exists, err := workspace.local.Exists() if err != nil { return err } - projectKeys, err := workspace.projectKeys() + synced, err := workspace.syncedResources() if err != nil { return err } - if !exists && len(projectKeys) == 0 { - if err := runner.bootstrap(runner.bootstrapOptions(options, workspace, true, action.DryRun, nil)); err != nil { + if !exists && len(synced) == 0 { + add := AddAction{Filter: action.Filter, DryRun: action.DryRun} + if err := runner.bootstrap(runner.bootstrapOptions(options, workspace, true, add)); err != nil { return err } if !action.Watch { @@ -138,11 +140,11 @@ func (runner Runner) runAdd(options Options, workspace syncWorkspace, action Add if err != nil { return err } - return runner.bootstrap(runner.bootstrapOptions(options, workspace, !exists, action.DryRun, action.Variations)) + return runner.bootstrap(runner.bootstrapOptions(options, workspace, !exists, action)) } func (runner Runner) runDetach(options Options, workspace syncWorkspace, action DetachAction) error { - projectKeys, err := workspace.projectKeys() + synced, err := workspace.syncedResources() if err != nil { return err } @@ -150,10 +152,11 @@ func (runner Runner) runDetach(options Options, workspace syncWorkspace, action RepositoryRoot: workspace.root, Store: workspace.local, Baselines: workspace.baselines, - ProjectKeys: projectKeys, + ProjectKeys: projectKeys(synced, syncdomain.Filter{}), Input: options.Input, Output: options.Output, - Selections: action.Variations, + Filter: action.Filter, + Selections: selections(action.Variations, action.Filter), NoInput: options.NoInput, Archive: action.Archive, Archiver: runner.api(options), @@ -174,6 +177,7 @@ func (runner Runner) runLink(options Options, workspace syncWorkspace, action Li Format: action.Format, Input: options.Input, Output: options.Output, + ProjectKey: action.ProjectKey, Target: action.Target, NoInput: options.NoInput, }) @@ -197,6 +201,7 @@ func (runner Runner) runAttach(options Options, workspace syncWorkspace, action attach := attachOptions{ RepositoryRoot: workspace.root, + ProjectKey: action.ProjectKey, Kind: action.Kind, Key: action.Key, // The attach forms write to the output stream, so check that stream. @@ -214,12 +219,7 @@ func (runner Runner) runAttach(options Options, workspace syncWorkspace, action return err == nil, err } -func (runner Runner) bootstrapOptions( - options Options, - workspace syncWorkspace, - initial, dryRun bool, - selections []syncdomain.ResourceID, -) syncbootstrap.Options { +func (runner Runner) bootstrapOptions(options Options, workspace syncWorkspace, initial bool, action AddAction) syncbootstrap.Options { api := runner.api(options) return syncbootstrap.Options{ Catalog: api, @@ -229,8 +229,10 @@ func (runner Runner) bootstrapOptions( Input: options.Input, Output: options.Output, Initial: initial, - DryRun: dryRun, - Selections: selections, + DryRun: action.DryRun, + ProjectKey: action.Filter.Project, + ConfigKey: action.Filter.ConfigKey, + Selections: selections(action.Variations, action.Filter), NoInput: options.NoInput, Diff: options.Diff, } diff --git a/internal/sync/prompt/state.go b/internal/sync/prompt/state.go index 35ac6480d..f86f61e89 100644 --- a/internal/sync/prompt/state.go +++ b/internal/sync/prompt/state.go @@ -23,19 +23,23 @@ type workspaceState struct { } // loadState reads the baseline, the local files, and LaunchDarkly, and builds -// the plan. -func (workspace syncWorkspace) loadState(client syncapi.Client) (workspaceState, error) { - projectKeys, err := workspace.projectKeys() +// the plan of the resources that the filter keeps. +func (workspace syncWorkspace) loadState(client syncapi.Client, filter syncdomain.Filter) (workspaceState, error) { + synced, err := workspace.syncedResources() if err != nil { return workspaceState{}, err } + if err := checkFilter(filter, synced); err != nil { + return workspaceState{}, err + } + projectKeys := projectKeys(synced, filter) // The lock is the common ancestor in a three-way comparison of the local // files and LaunchDarkly. baseline, err := workspace.baselines.Load(projectKeys) if err != nil { return workspaceState{}, err } - plan, localFiles, err := loadWorkspacePlan(workspace.root, baseline, client) + plan, localFiles, err := loadWorkspacePlan(workspace.root, baseline, client, filter) if err != nil { return workspaceState{}, err } @@ -44,31 +48,40 @@ func (workspace syncWorkspace) loadState(client syncapi.Client) (workspaceState, // loadWorkspacePlan reads the local variations and their LaunchDarkly // versions, and compares both with the lock. It also returns the local -// variations as their files store them. +// variations as their files store them. It drops each variation that the +// filter does not keep before it reads LaunchDarkly. func loadWorkspacePlan( repositoryRoot string, baseline syncmanifest.Baseline, client syncapi.Client, + filter syncdomain.Filter, ) (Plan, map[ResourceID]syncdomain.SyncedResource, error) { localFiles, err := compileWorkspace(repositoryRoot) if err != nil { return Plan{}, nil, err } + localFiles = slices.DeleteFunc(localFiles, func(resource syncdomain.SyncedResource) bool { + return !filter.Includes(resource.ID()) + }) local, err := canonicalizeLocalVariationModels(localFiles, client.ModelConfig) if err != nil { return Plan{}, nil, err } + tracked := syncmanifest.New() + for _, resource := range baseline.Lock.Resources { + if resource.ResourceKind == syncdomain.KindVariation && filter.Includes(resource.ID()) { + tracked.Resources = append(tracked.Resources, resource) + } + } localFilesByID := make(map[ResourceID]syncdomain.SyncedResource, len(localFiles)) - ids := make(map[ResourceID]struct{}, len(localFiles)+len(baseline.Lock.Resources)) + ids := make(map[ResourceID]struct{}, len(localFiles)+len(tracked.Resources)) for _, resource := range localFiles { localFilesByID[resource.ID()] = resource ids[resource.ID()] = struct{}{} } - for _, resource := range baseline.Lock.Resources { - if resource.ResourceKind == syncdomain.KindVariation { - ids[resource.ID()] = struct{}{} - } + for _, resource := range tracked.Resources { + ids[resource.ID()] = struct{}{} } attachments := newAttachmentCache(client) @@ -80,7 +93,7 @@ func loadWorkspacePlan( } server[id] = resource } - plan := BuildPlan(baseline.Lock, local, server) + plan := BuildPlan(tracked, local, server) for index := range plan.Resources { plan.Resources[index].SyncedElsewhere = baseline.Stale(plan.Resources[index].ID) planRestore(repositoryRoot, baseline, &plan.Resources[index]) @@ -146,10 +159,11 @@ func readServerResource(client syncapi.Client, attachments *attachmentCache, id return resource, nil } -// projectKeys returns each project that has a local sync file or an entry in -// the sync.lock file. A project whose files are all missing is still in the -// lock, so sync can restore its files. -func (workspace syncWorkspace) projectKeys() ([]string, error) { +// syncedResources returns the identity of each resource that has a local sync +// file or an entry in the sync.lock file. It reads only local files, so that a +// filter that names a resource that is not synced fails before any +// LaunchDarkly read. +func (workspace syncWorkspace) syncedResources() ([]ResourceID, error) { files, err := synclocal.SourceFiles(workspace.root) if err != nil { return nil, err @@ -159,14 +173,56 @@ func (workspace syncWorkspace) projectKeys() ([]string, error) { return nil, err } - projectKeys := lock.ProjectKeys() + var synced []ResourceID + for _, resource := range lock.Resources { + synced = append(synced, resource.ID()) + } for _, file := range files { if id, ok := synclocal.ParseManagedPath(file); ok { - projectKeys = append(projectKeys, id.ProjectKey) + synced = append(synced, id) } } - slices.Sort(projectKeys) - return slices.Compact(projectKeys), nil + return synced, nil +} + +// projectKeys returns each project of a synced resource that the filter keeps. +// A project whose files are all missing is still in the lock, so sync can +// restore its files. +func projectKeys(synced []ResourceID, filter syncdomain.Filter) []string { + var keys []string + for _, id := range synced { + if filter.Includes(id) { + keys = append(keys, id.ProjectKey) + } + } + slices.Sort(keys) + return slices.Compact(keys) +} + +// checkFilter returns an error when the filter names a project, a config, or +// a variation that has no local file and no sync.lock entry, so that a typo +// never gives an empty plan that looks like success. +func checkFilter(filter syncdomain.Filter, synced []ResourceID) error { + if filter.IsZero() { + return nil + } + if !slices.ContainsFunc(synced, func(id ResourceID) bool { return id.ProjectKey == filter.Project }) { + return fmt.Errorf("project %q has no synced resources in this repository. "+ + "To start, run ldcli sync prompts add --project %s.", filter.Project, filter.Project) + } + for _, key := range filter.Keys { + if id := syncdomain.VariationID(filter.Project, filter.ConfigKey, key); !slices.Contains(synced, id) { + return fmt.Errorf("%s is not synced in this repository. To start syncing it, "+ + "run ldcli sync prompts add --project %s --config-key %s --key %s.", + id.Label(), filter.Project, filter.ConfigKey, key) + } + } + if !slices.ContainsFunc(synced, filter.Includes) { + return fmt.Errorf("config %q in project %q has no synced variations in this repository. "+ + "To start, run ldcli sync prompts add --project %s --config-key %s.", + filter.ConfigKey, filter.Project, filter.Project, filter.ConfigKey) + } + return nil } // compileWorkspace reads the local variations. A workspace without a managed diff --git a/internal/sync/prompt/sync.go b/internal/sync/prompt/sync.go index 70a213af2..8588ecd65 100644 --- a/internal/sync/prompt/sync.go +++ b/internal/sync/prompt/sync.go @@ -31,7 +31,7 @@ func (runner Runner) runWorkspaceSync(options Options, workspace syncWorkspace, watched = &watchedSources{watcher: watcher, snapshot: snapshot, debounce: watchDebounce} } - reviewed, err := workspace.loadState(client) + reviewed, err := workspace.loadState(client, options.syncAction().Filter) if err != nil { return err } @@ -67,7 +67,7 @@ func (runner Runner) runWorkspaceSync(options Options, workspace syncWorkspace, } // Read both sides again, so that no action uses state from before the review. - current, err := workspace.loadState(client) + current, err := workspace.loadState(client, options.syncAction().Filter) if err != nil { return err } @@ -103,7 +103,9 @@ func (runner Runner) applyPlan( interactive bool, ) error { plan := applyConflictResolutions(current.plan, resolutions) - outcomes, next, err := executePlan(workspace.root, workspace.local, client, current.baseline.Lock, plan, current.localFiles) + outcomes, next, err := executePlan( + workspace.root, workspace.local, client, current.baseline.Lock, plan, current.localFiles, options.syncAction().Filter, + ) failures := []error{err} if _, err := workspace.baselines.Save(current.baseline, next); err != nil { diff --git a/internal/sync/scope.go b/internal/sync/scope.go new file mode 100644 index 000000000..238a302ca --- /dev/null +++ b/internal/sync/scope.go @@ -0,0 +1,106 @@ +package sync + +import ( + "errors" + "fmt" + "slices" + "strings" +) + +// Filter is the resources that the --project, --config-key, and --key flags +// name. A sync keeps only those resources. An add or a detach uses them +// instead of asking. The zero Filter names every resource. +type Filter struct { + Project string + ConfigKey string + // Keys are the keys of resources of one kind, for example variation keys. + Keys []string +} + +// IsZero reports whether no target flag is set. +func (filter Filter) IsZero() bool { + return filter.Project == "" && filter.ConfigKey == "" && len(filter.Keys) == 0 +} + +// Validate makes sure that each flag has its parent flag, and that no value is +// blank, unsafe, or repeated. kind is the kind of resource that --key names. +func (filter Filter) Validate(kind Kind) error { + if err := checkFlagValue("--project", filter.Project); err != nil { + return err + } + if err := checkFlagValue("--config-key", filter.ConfigKey); err != nil { + return err + } + for _, key := range filter.Keys { + if key == "" { + return errors.New("--key cannot be blank") + } + if err := checkFlagValue("--key", key); err != nil { + return err + } + } + + switch { + case len(filter.Keys) != 0 && filter.Project == "": + return errors.New("--key needs --project") + case filter.ConfigKey != "" && filter.Project == "": + return errors.New("--config-key needs --project") + case len(filter.Keys) != 0 && kind == KindVariation && filter.ConfigKey == "": + return errors.New("--key needs --config-key. In sync prompts, --key is a variation key.") + } + + ids := filter.IDs(kind) + for index, id := range ids { + if slices.Contains(ids[:index], id) { + return fmt.Errorf("%s was selected more than once", id.Label()) + } + } + return nil +} + +// IDs returns the resources of kind that --key names. +func (filter Filter) IDs(kind Kind) []ResourceID { + ids := make([]ResourceID, 0, len(filter.Keys)) + for _, key := range filter.Keys { + if kind == KindVariation { + ids = append(ids, VariationID(filter.Project, filter.ConfigKey, key)) + } else { + ids = append(ids, ResourceID{Kind: kind, ProjectKey: filter.Project, LookupKey: key}) + } + } + return ids +} + +// Includes reports whether the filter keeps the resource. --config-key keeps +// only variations of that config. +func (filter Filter) Includes(id ResourceID) bool { + if filter.Project != "" && id.ProjectKey != filter.Project { + return false + } + key := id.LookupKey + if id.Kind == KindVariation { + configKey, variationKey, err := id.VariationKeys() + if err != nil || filter.ConfigKey != "" && configKey != filter.ConfigKey { + return false + } + key = variationKey + } else if filter.ConfigKey != "" { + return false + } + return len(filter.Keys) == 0 || slices.Contains(filter.Keys, key) +} + +// checkFlagValue rejects a blank or unsafe flag value. An empty value is a +// flag that is not set. +func checkFlagValue(flag, value string) error { + switch { + case value == "": + return nil + case strings.TrimSpace(value) == "": + return fmt.Errorf("%s cannot be blank", flag) + } + if err := ValidateKey(value); err != nil { + return fmt.Errorf("invalid %s %q: %w", flag, value, err) + } + return nil +} diff --git a/internal/sync/scope_test.go b/internal/sync/scope_test.go new file mode 100644 index 000000000..50cd50a57 --- /dev/null +++ b/internal/sync/scope_test.go @@ -0,0 +1,80 @@ +package sync + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestFilterValidateNeedsParentsAndRejectsBadValues(t *testing.T) { + tests := map[string]struct { + filter Filter + want string + }{ + "key without project": {Filter{Keys: []string{"default"}}, "--key needs --project"}, + "config without project": {Filter{ConfigKey: "support"}, "--config-key needs --project"}, + "key without config": { + Filter{Project: "demo", Keys: []string{"default"}}, + "--key needs --config-key. In sync prompts, --key is a variation key.", + }, + "blank project": {Filter{Project: " "}, "--project cannot be blank"}, + "blank key": {Filter{Project: "demo", ConfigKey: "support", Keys: []string{""}}, "--key cannot be blank"}, + "unsafe key": { + Filter{Project: "demo", ConfigKey: "support", Keys: []string{"a/b"}}, + `invalid --key "a/b": must be a single path segment`, + }, + "repeated key": { + Filter{Project: "demo", ConfigKey: "support", Keys: []string{"default", "default"}}, + "variation demo/support/default was selected more than once", + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + require.EqualError(t, test.filter.Validate(KindVariation), test.want) + }) + } + require.NoError(t, Filter{}.Validate(KindVariation)) + require.NoError(t, Filter{Project: "demo", ConfigKey: "support", Keys: []string{"a", "b"}}.Validate(KindVariation)) +} + +func TestFilterIncludesOnlyTheNamedResources(t *testing.T) { + first := VariationID("demo", "support", "first") + second := VariationID("demo", "support", "second") + otherConfig := VariationID("demo", "chat", "first") + otherProject := VariationID("other", "support", "first") + tool := ResourceID{Kind: KindTool, ProjectKey: "demo", LookupKey: "first"} + + tests := map[string]struct { + filter Filter + want []ResourceID + }{ + "empty": {Filter{}, []ResourceID{first, second, otherConfig, otherProject, tool}}, + "project": {Filter{Project: "demo"}, []ResourceID{first, second, otherConfig, tool}}, + "config": {Filter{Project: "demo", ConfigKey: "support"}, []ResourceID{first, second}}, + "key": {Filter{Project: "demo", ConfigKey: "support", Keys: []string{"first"}}, []ResourceID{first}}, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + var included []ResourceID + for _, id := range []ResourceID{first, second, otherConfig, otherProject, tool} { + if test.filter.Includes(id) { + included = append(included, id) + } + } + assert.Equal(t, test.want, included) + }) + } +} + +func TestFilterIDsNamesEachKey(t *testing.T) { + filter := Filter{Project: "demo", ConfigKey: "support", Keys: []string{"first", "second"}} + + assert.Equal(t, []ResourceID{ + VariationID("demo", "support", "first"), VariationID("demo", "support", "second"), + }, filter.IDs(KindVariation)) + assert.True(t, Filter{}.IsZero()) + assert.False(t, filter.IsZero()) +}