Skip to content
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

Add option --allow-change-package-name in the package installed update operation #1671

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Add option --allow-change-package-name in the package installed update operation #1671

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 11 additions & 3 deletions cli/pkg/kctrl/cmd/package/installed/create_or_update.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ type CreateOrUpdateOptions struct {
values bool
serviceAccountName string

rename bool

install bool

DryRun bool
Expand Down Expand Up @@ -195,6 +197,7 @@ func NewUpdateCmd(o *CreateOrUpdateOptions, flagsFactory cmdcore.FlagsFactory) *
cmd.Flags().StringVarP(&o.version, "version", "v", "", "Set package version")
cmd.Flags().StringVar(&o.valuesFile, "values-file", "", "The path to the configuration values file, optional")
cmd.Flags().BoolVar(&o.values, "values", true, "Add or keep values supplied to package install, optional")
cmd.Flags().BoolVar(&o.rename, "allow-change-package-name", false, "Allow to use different package name, optional")
Copy link

Choose a reason for hiding this comment

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

please change the flag name to allow-package-name-change or rename-package ?


o.WaitFlags.Set(cmd, flagsFactory, &cmdcore.WaitFlagsOpts{
AllowDisableWait: true,
Expand Down Expand Up @@ -767,13 +770,18 @@ func (o *CreateOrUpdateOptions) preparePackageInstallForUpdate(pkgInstall *kcpkg
return nil, fmt.Errorf("Failed to update package '%s' as no existing package reference/version was found in the package install", o.Name)
}

// if o.rename is true, we allow changing the package name in the package installed update operation.
// If o.PackageName is provided by the user (via --package flag), verify that the package name in PackageInstall matches it.
// This will prevent the users from accidentally overwriting an installed package with another package content due to choosing a pre-existing name for the package isntall.
// Otherwise if o.PackageName is not provided, fill it from the installed package spec
if o.packageName != "" && updatedPkgInstall.Spec.PackageRef.RefName != o.packageName {
return nil, fmt.Errorf("Installed package '%s' is already associated with package '%s'", o.Name, updatedPkgInstall.Spec.PackageRef.RefName)
if !o.rename {
Copy link

Choose a reason for hiding this comment

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

nit:
could you please reverse the if condition?

if o.rename {
    updatedPkgInstall.Spec.PackageRef.RefName = o.packageName
} else {
    if o.packageName != "" && updatedPkgInstall.Spec.PackageRef.RefName != o.packageName {
        return nil, fmt.Errorf("Installed package '%s' is already associated with package '%s'", o.Name, updatedPkgInstall.Spec.PackageRef.RefName)
    }
    o.packageName = updatedPkgInstall.Spec.PackageRef.RefName
}

if o.packageName != "" && updatedPkgInstall.Spec.PackageRef.RefName != o.packageName {
return nil, fmt.Errorf("Installed package '%s' is already associated with package '%s'", o.Name, updatedPkgInstall.Spec.PackageRef.RefName)
}
o.packageName = updatedPkgInstall.Spec.PackageRef.RefName
} else {
updatedPkgInstall.Spec.PackageRef.RefName = o.packageName
}
o.packageName = updatedPkgInstall.Spec.PackageRef.RefName

// If o.Version is provided by the user (via --version flag), set the version in PackageInstall to this version
// Otherwise if o.Version is not provided, fill it from the installed package spec
Expand Down
Loading