From 1c7ccf3fe05f740604f36bc73b044f92940cdbd7 Mon Sep 17 00:00:00 2001 From: Soumik Majumder Date: Fri, 30 Aug 2024 15:46:15 +0530 Subject: [PATCH] Make ownership override allowed apps flag dangerous Signed-off-by: Soumik Majumder --- pkg/kapp/cmd/app/deploy_flags.go | 4 +- pkg/kapp/resources/labeled_resources.go | 9 ++-- test/e2e/selective_ownership_override_test.go | 53 +++++++++++++++---- 3 files changed, 51 insertions(+), 15 deletions(-) diff --git a/pkg/kapp/cmd/app/deploy_flags.go b/pkg/kapp/cmd/app/deploy_flags.go index 939050553..09dd2e150 100644 --- a/pkg/kapp/cmd/app/deploy_flags.go +++ b/pkg/kapp/cmd/app/deploy_flags.go @@ -49,8 +49,8 @@ func (s *DeployFlags) Set(cmd *cobra.Command) { 100, "Concurrency to check for existing non-labeled resources") cmd.Flags().BoolVar(&s.OverrideOwnershipOfExistingResources, "dangerous-override-ownership-of-existing-resources", false, "Steal existing resources from another app") - cmd.Flags().StringSliceVar(&s.OwnershipOverrideAllowedApps, "ownership-override-allowed-apps", nil, - "Specify existing apps in the same namespace that existing resources can be stolen from if --dangerous-override-ownership-of-existing-resources is set") + cmd.Flags().StringSliceVar(&s.OwnershipOverrideAllowedApps, "dangerous-ownership-override-allowed-apps", nil, + "Specify existing apps in the same namespace that existing resources can be stolen from") cmd.Flags().BoolVar(&s.DefaultLabelScopingRules, "default-label-scoping-rules", true, "Use default label scoping rules") diff --git a/pkg/kapp/resources/labeled_resources.go b/pkg/kapp/resources/labeled_resources.go index b334bf44c..00c59c1e3 100644 --- a/pkg/kapp/resources/labeled_resources.go +++ b/pkg/kapp/resources/labeled_resources.go @@ -134,8 +134,7 @@ func (a *LabeledResources) AllAndMatching(newResources []Resource, opts AllAndMa } } - if len(nonLabeledResources) > 0 && (!opts.SkipResourceOwnershipCheck || - (opts.SkipResourceOwnershipCheck && len(opts.SkipOwnershipCheckAllowedApps) > 0)) { + if len(nonLabeledResources) > 0 && !opts.SkipResourceOwnershipCheck { resourcesForCheck := a.resourcesForOwnershipCheck(newResources, nonLabeledResources) if len(resourcesForCheck) > 0 { err := a.checkResourceOwnership(resourcesForCheck, opts) @@ -185,14 +184,16 @@ func (a *LabeledResources) checkResourceOwnership(resources []Resource, opts All var errs []error labelValAppMap := map[string]string{} - if len(opts.SkipOwnershipCheckAllowedApps) > 0 && opts.SkipResourceOwnershipCheck { + isSelectiveOwnershipOverride := len(opts.SkipOwnershipCheckAllowedApps) > 0 + if isSelectiveOwnershipOverride { labelValAppMap = opts.LabelValAppMapResolverFunc() } for _, res := range resources { if val, found := res.Labels()[expectedLabelKey]; found { ownershipOverrideAllowed := false - if opts.SkipResourceOwnershipCheck { + + if isSelectiveOwnershipOverride { ownershipOverrideAllowed = a.ownershipOverrideAllowed(labelValAppMap, res, expectedLabelKey, opts.SkipOwnershipCheckAllowedApps) } diff --git a/test/e2e/selective_ownership_override_test.go b/test/e2e/selective_ownership_override_test.go index 68ff6ba02..464912fa6 100644 --- a/test/e2e/selective_ownership_override_test.go +++ b/test/e2e/selective_ownership_override_test.go @@ -11,6 +11,48 @@ import ( "github.com/stretchr/testify/require" ) +func TestOwnershipOverride(t *testing.T) { + env := BuildEnv(t) + logger := Logger{} + kapp := Kapp{t, env.Namespace, env.KappBinaryPath, logger} + + const existingAppName = "existing-app" + const newAppName = "new-app" + + resourceYAML := ` +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: cm-%s +data: + foo: bar +` + + cleanUp := func() { + kapp.Run([]string{"delete", "-a", existingAppName}) + kapp.Run([]string{"delete", "-a", newAppName}) + } + cleanUp() + defer cleanUp() + + logger.Section("deploy existing app", func() { + kapp.RunWithOpts([]string{"deploy", "-a", existingAppName, "-f", "-"}, RunOpts{StdinReader: strings.NewReader(fmt.Sprintf(resourceYAML, "1"))}) + }) + + logger.Section("deploy new app with ownership overrides", func() { + resourcesString := fmt.Sprintf("%s", fmt.Sprintf(resourceYAML, "1")) + // Check for ownership errors without flag + _, err := kapp.RunWithOpts([]string{"deploy", "-a", newAppName, "-f", "-"}, RunOpts{StdinReader: strings.NewReader(resourcesString), AllowError: true}) + require.Error(t, err) + require.Contains(t, err.Error(), existingAppName) + + // Check for successful ownership override + kapp.RunWithOpts([]string{"deploy", "-a", newAppName, "-f", "-", "--dangerous-override-ownership-of-existing-resources"}, + RunOpts{StdinReader: strings.NewReader(resourcesString)}) + }) +} + func TestSelectiveOwnershipOverride(t *testing.T) { env := BuildEnv(t) logger := Logger{} @@ -51,22 +93,15 @@ data: require.Contains(t, err.Error(), existingAppName1) require.Contains(t, err.Error(), existingAppName2) - // Test with override scoped while override is disallowed - _, err = kapp.RunWithOpts([]string{"deploy", "-a", newAppName, "-f", "-", "--ownership-override-allowed-apps", existingAppName1}, - RunOpts{StdinReader: strings.NewReader(resourcesString), AllowError: true}) - require.Error(t, err) - require.Contains(t, err.Error(), existingAppName1) - require.Contains(t, err.Error(), existingAppName2) - // Test with override scoped to single existing app - _, err = kapp.RunWithOpts([]string{"deploy", "-a", newAppName, "-f", "-", "--dangerous-override-ownership-of-existing-resources", "--ownership-override-allowed-apps", existingAppName1}, + _, err = kapp.RunWithOpts([]string{"deploy", "-a", newAppName, "-f", "-", "--dangerous-ownership-override-allowed-apps", existingAppName1}, RunOpts{StdinReader: strings.NewReader(resourcesString), AllowError: true}) require.Error(t, err) require.NotContains(t, err.Error(), existingAppName1) require.Contains(t, err.Error(), existingAppName2) // Test with override scoped to both existing app - kapp.RunWithOpts([]string{"deploy", "-a", newAppName, "-f", "-", "--dangerous-override-ownership-of-existing-resources", "--ownership-override-allowed-apps", fmt.Sprintf("%s,%s", existingAppName1, existingAppName2)}, + kapp.RunWithOpts([]string{"deploy", "-a", newAppName, "-f", "-", "--dangerous-ownership-override-allowed-apps", fmt.Sprintf("%s,%s", existingAppName1, existingAppName2)}, RunOpts{StdinReader: strings.NewReader(resourcesString)}) }) }