Skip to content

Add opt-in --crd flag to install and uninstall commands #470

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kelos-bot wants to merge 2 commits into
base: main
Choose a base branch
from
+78 −13
Open

Add opt-in --crd flag to install and uninstall commands #470

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5fd6d7c
Add --crd flag to install and uninstall commands
gjkim42 Feb 27, 2026
6b358e6
Change --crd flag to opt-in (default false)
gjkim42 Feb 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions internal/cli/dryrun_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -581,8 +581,8 @@ func TestInstallCommand_DryRun(t *testing.T) {
}
})

if !strings.Contains(output, "CustomResourceDefinition") {
t.Errorf("expected CRD manifest in dry-run output, got:\n%s", output[:min(len(output), 500)])
if strings.Contains(output, "CustomResourceDefinition") {
t.Errorf("expected no CRD manifest in default dry-run output, got:\n%s", output[:min(len(output), 500)])
}
if !strings.Contains(output, "Deployment") {
t.Errorf("expected Deployment manifest in dry-run output, got:\n%s", output[:min(len(output), 500)])
Expand Down
34 changes: 23 additions & 11 deletions internal/cli/install.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,11 @@ func newInstallCommand(cfg *ClientConfig) *cobra.Command {
var dryRun bool
var flagVersion string
var imagePullPolicy string
var crd bool

cmd := &cobra.Command{
Use: "install",
Short: "Install kelos CRDs and controller into the cluster",
Short: "Install kelos controller into the cluster",
Args: cobra.NoArgs,
RunE: func(cmd *cobra.Command, args []string) error {
if flagVersion != "" {
Expand All @@ -46,10 +47,12 @@ func newInstallCommand(cfg *ClientConfig) *cobra.Command {
}

if dryRun {
if _, err := os.Stdout.Write(manifests.InstallCRD); err != nil {
return err
if crd {
if _, err := os.Stdout.Write(manifests.InstallCRD); err != nil {
return err
}
fmt.Fprintln(os.Stdout, "---")
}
fmt.Fprintln(os.Stdout, "---")
_, err := os.Stdout.Write(controllerManifest)
return err
}
Expand All @@ -70,9 +73,11 @@ func newInstallCommand(cfg *ClientConfig) *cobra.Command {

ctx := cmd.Context()

fmt.Fprintf(os.Stdout, "Installing kelos CRDs\n")
if err := applyManifests(ctx, dc, dyn, manifests.InstallCRD); err != nil {
return fmt.Errorf("installing CRDs: %w", err)
if crd {
fmt.Fprintf(os.Stdout, "Installing kelos CRDs\n")
if err := applyManifests(ctx, dc, dyn, manifests.InstallCRD); err != nil {
return fmt.Errorf("installing CRDs: %w", err)
}
}

fmt.Fprintf(os.Stdout, "Installing kelos controller (version: %s)\n", version.Version)
Expand All @@ -88,6 +93,7 @@ func newInstallCommand(cfg *ClientConfig) *cobra.Command {
cmd.Flags().BoolVar(&dryRun, "dry-run", false, "print the manifests that would be applied without installing")
cmd.Flags().StringVar(&flagVersion, "version", "", "override the version used for image tags (defaults to the binary version)")
cmd.Flags().StringVar(&imagePullPolicy, "image-pull-policy", "", "set imagePullPolicy on controller containers (e.g. Always, IfNotPresent, Never)")
cmd.Flags().BoolVar(&crd, "crd", false, "install CRDs along with the controller")
Copy link

@cubic-dev-ai cubic-dev-ai bot Feb 28, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P0: The --crd flag default is changed from true to false, which is a breaking change that contradicts the PR description (which says default is true) and the README Quick Start guide (which says kelos install installs CRDs). Users running kelos install will silently skip CRD installation. If the intent is truly default=false, the README must be updated; if not, this default should be true.

(Based on your team's feedback about keeping documentation and config updated with changes.)

View Feedback

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At internal/cli/install.go, line 96:

<comment>The `--crd` flag default is changed from `true` to `false`, which is a breaking change that contradicts the PR description (which says default is `true`) and the README Quick Start guide (which says `kelos install` installs CRDs). Users running `kelos install` will silently skip CRD installation. If the intent is truly `default=false`, the README must be updated; if not, this default should be `true`.

(Based on your team's feedback about keeping documentation and config updated with changes.) </comment>

<file context>
@@ -95,7 +93,7 @@ func newInstallCommand(cfg *ClientConfig) *cobra.Command {
 	cmd.Flags().StringVar(&flagVersion, "version", "", "override the version used for image tags (defaults to the binary version)")
 	cmd.Flags().StringVar(&imagePullPolicy, "image-pull-policy", "", "set imagePullPolicy on controller containers (e.g. Always, IfNotPresent, Never)")
-	cmd.Flags().BoolVar(&crd, "crd", true, "install CRDs (set to false to skip CRD installation)")
+	cmd.Flags().BoolVar(&crd, "crd", false, "install CRDs along with the controller")
 
 	return cmd
</file context>
Suggested change
cmd.Flags().BoolVar(&crd, "crd", false, "install CRDs along with the controller")
cmd.Flags().BoolVar(&crd, "crd", true, "install CRDs (set to false to skip CRD installation)")
Fix with Cubic


return cmd
}
Expand Down Expand Up @@ -134,9 +140,11 @@ func withImagePullPolicy(data []byte, policy string) []byte {
}

func newUninstallCommand(cfg *ClientConfig) *cobra.Command {
var crd bool

cmd := &cobra.Command{
Use: "uninstall",
Short: "Uninstall kelos controller and CRDs from the cluster",
Short: "Uninstall kelos controller from the cluster",
Args: cobra.NoArgs,
RunE: func(cmd *cobra.Command, args []string) error {
restConfig, _, err := cfg.resolveConfig()
Expand All @@ -160,16 +168,20 @@ func newUninstallCommand(cfg *ClientConfig) *cobra.Command {
return fmt.Errorf("removing controller: %w", err)
}

fmt.Fprintf(os.Stdout, "Removing kelos CRDs\n")
if err := deleteManifests(ctx, dc, dyn, manifests.InstallCRD); err != nil {
return fmt.Errorf("removing CRDs: %w", err)
if crd {
fmt.Fprintf(os.Stdout, "Removing kelos CRDs\n")
if err := deleteManifests(ctx, dc, dyn, manifests.InstallCRD); err != nil {
return fmt.Errorf("removing CRDs: %w", err)
}
}

fmt.Fprintf(os.Stdout, "Kelos uninstalled successfully\n")
return nil
},
}

cmd.Flags().BoolVar(&crd, "crd", false, "remove CRDs along with the controller")
Copy link

@cubic-dev-ai cubic-dev-ai bot Feb 28, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The --crd flag default is changed from true to false for uninstall, contradicting the PR description which says default is true. While default=false is arguably the safer choice for uninstall (since deleting CRDs cascades to all custom resources), this contradicts the stated design. If default=false is intentional, the PR description needs correction; if not, restore true.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At internal/cli/install.go, line 183:

<comment>The `--crd` flag default is changed from `true` to `false` for uninstall, contradicting the PR description which says default is `true`. While `default=false` is arguably the safer choice for uninstall (since deleting CRDs cascades to all custom resources), this contradicts the stated design. If `default=false` is intentional, the PR description needs correction; if not, restore `true`.</comment>

<file context>
@@ -175,16 +173,14 @@ func newUninstallCommand(cfg *ClientConfig) *cobra.Command {
 	}
 
-	cmd.Flags().BoolVar(&crd, "crd", true, "remove CRDs (set to false to keep CRDs in the cluster)")
+	cmd.Flags().BoolVar(&crd, "crd", false, "remove CRDs along with the controller")
 
 	return cmd
</file context>
Fix with Cubic


return cmd
}

Expand Down
53 changes: 53 additions & 0 deletions internal/cli/install_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,59 @@ func TestWithImagePullPolicy(t *testing.T) {
}
}

func TestInstallCommand_CRDFlagDefault(t *testing.T) {
cmd := NewRootCommand()
cmd.SetArgs([]string{"install", "--dry-run"})

output := captureStdout(t, func() {
if err := cmd.Execute(); err != nil {
t.Fatalf("unexpected error: %v", err)
}
})

if strings.Contains(output, "CustomResourceDefinition") {
t.Errorf("expected no CRD manifest in default output, got:\n%s", output[:min(len(output), 500)])
}
if !strings.Contains(output, "Deployment") {
t.Errorf("expected controller manifest in output, got:\n%s", output[:min(len(output), 500)])
}
}

func TestInstallCommand_CRDFlagTrue(t *testing.T) {
cmd := NewRootCommand()
cmd.SetArgs([]string{"install", "--dry-run", "--crd"})

output := captureStdout(t, func() {
if err := cmd.Execute(); err != nil {
t.Fatalf("unexpected error: %v", err)
}
})

if !strings.Contains(output, "CustomResourceDefinition") {
t.Errorf("expected CRD manifest when --crd is set, got:\n%s", output[:min(len(output), 500)])
}
if !strings.Contains(output, "Deployment") {
t.Errorf("expected controller manifest in output, got:\n%s", output[:min(len(output), 500)])
}
}

func TestUninstallCommand_CRDFlagAccepted(t *testing.T) {
cmd := NewRootCommand()
cmd.SetArgs([]string{
"uninstall",
"--crd",
"--kubeconfig", "/nonexistent/path/kubeconfig",
})
err := cmd.Execute()
if err == nil {
t.Fatal("expected uninstall to fail with invalid kubeconfig")
}
// The flag should be accepted; the error should be about kubeconfig, not an unknown flag.
if strings.Contains(err.Error(), "unknown flag") {
t.Fatalf("--crd flag should be accepted, got: %v", err)
}
}

func TestWithImagePullPolicy_EmbeddedController(t *testing.T) {
result := withImagePullPolicy(manifests.InstallController, "IfNotPresent")
if !bytes.Contains(result, []byte("imagePullPolicy: IfNotPresent")) {
Expand Down
Loading