Fixed a bug where environments could not be deleted. (#1623)

This commit fixes a bug where environments could not be deleted if
the organization they reside in has one or more assets.

Assets do not exist within environments, so there is no reason to
consider them when deleting environments.

This commit also stops the environment store from reporting an
error when an environment that doesn't exist is deleted.

Closes #1567

Signed-off-by: Eric Chlebek <[email protected]>

CHANGELOG.md

@@ -56,6 +56,8 @@ characters and therefore fixing a bug in sensuctl
 - API requests that result in a 404 response are now logged
 - Fixed a bug where only a single resource could be created with
 `sensuctl create` at a time.
+- Fixed a bug where environments couldn't be deleted if there was an asset in
+the organization they reside in.
 - Dashboard's backend reverse proxy now works with TLS certs are configured.
 
 ### Removed

backend/store/etcd/environment_store.go

@@ -36,7 +36,6 @@ func (s *Store) DeleteEnvironment(ctx context.Context, env *types.Environment) e
 	getresp, err := s.client.Txn(ctx).Then(
 		v3.OpGet(checkKeyBuilder.WithContext(ctx).Build(), v3.WithPrefix(), v3.WithCountOnly()),
 		v3.OpGet(entityKeyBuilder.WithContext(ctx).Build(), v3.WithPrefix(), v3.WithCountOnly()),
-		v3.OpGet(assetKeyBuilder.WithContext(ctx).Build(), v3.WithPrefix(), v3.WithCountOnly()),
 		v3.OpGet(handlerKeyBuilder.WithContext(ctx).Build(), v3.WithPrefix(), v3.WithCountOnly()),
 		v3.OpGet(mutatorKeyBuilder.WithContext(ctx).Build(), v3.WithPrefix(), v3.WithCountOnly()),
 	).Commit()
@@ -45,7 +44,7 @@ func (s *Store) DeleteEnvironment(ctx context.Context, env *types.Environment) e
 	}
 	for _, r := range getresp.Responses {
 		if r.GetResponseRange().Count > 0 {
-			return errors.New("environment is not empty") // TODO
+			return errors.New("environment is not empty")
 		}
 	}
 
@@ -62,16 +61,8 @@ func (s *Store) DeleteEnvironment(ctx context.Context, env *types.Environment) e
 		}
 	}
 
-	resp, err := s.client.Delete(ctx, getEnvironmentsPath(org, env.Name), v3.WithPrefix())
-	if err != nil {
-		return err
-	}
-
-	if resp.Deleted != 1 {
-		return fmt.Errorf("environment %s/%s does not exist", org, env.Name)
-	}
-
-	return nil
+	_, err = s.client.Delete(ctx, getEnvironmentsPath(org, env.Name), v3.WithPrefix())
+	return err
 }
 
 // GetEnvironment returns a single environment

backend/store/etcd/environment_store_test.go

@@ -56,10 +56,6 @@ func TestEnvStorage(t *testing.T) {
 		err = store.DeleteEnvironment(ctx, env)
 		assert.NoError(t, err)
 
-		// Delete a missing org
-		err = store.DeleteEnvironment(ctx, &types.Environment{Organization: org, Name: "missing"})
-		assert.Error(t, err)
-
 		// Create a environment within a missing org
 		env.Organization = "missing"
 		err = store.UpdateEnvironment(ctx, env)
@@ -71,3 +67,38 @@ func TestEnvStorage(t *testing.T) {
 		assert.Equal(t, 1, len(envs))
 	})
 }
+
+func TestDeleteEnvironment_GH1567(t *testing.T) {
+	testWithEtcd(t, func(store store.Store) {
+		ctx := context.Background()
+		ctx = context.WithValue(ctx, types.OrganizationKey, "default")
+		ctx = context.WithValue(ctx, types.EnvironmentKey, "foo")
+
+		org := "default"
+		env := types.FixtureEnvironment("foo")
+		require.NoError(t, store.UpdateEnvironment(ctx, env))
+
+		asset := types.FixtureAsset("foo")
+		asset.Organization = org
+
+		require.NoError(t, store.UpdateAsset(ctx, asset))
+
+		// Assets do not apply to environments
+		require.NoError(t, store.DeleteEnvironment(ctx, env))
+	})
+}
+
+func TestDoubleDelete(t *testing.T) {
+	testWithEtcd(t, func(store store.Store) {
+		ctx := context.Background()
+		ctx = context.WithValue(ctx, types.OrganizationKey, "default")
+		ctx = context.WithValue(ctx, types.EnvironmentKey, "foo")
+
+		env := types.FixtureEnvironment("foo")
+		require.NoError(t, store.UpdateEnvironment(ctx, env))
+
+		// Second delete should succeed
+		require.NoError(t, store.DeleteEnvironment(ctx, env))
+		require.NoError(t, store.DeleteEnvironment(ctx, env))
+	})
+}