Skip to content

Commit

Permalink
fix(acl): allow data deletion for non-reserved predicates (#8937)
Browse files Browse the repository at this point in the history
when data (non-reserved predicates) is added on UIDs that belong
to groot user or guardian group, it is allowed. But when the same
data is deleted, that is not allowed. This PR allows deletion of
non-reserved predicates on special UIDs.

Closes: https://dgraph.atlassian.net/browse/DGRAPHCORE-355
  • Loading branch information
jbhamra1 authored and mangalaman93 committed Aug 21, 2023
1 parent 28ac6c2 commit c2e272b
Show file tree
Hide file tree
Showing 12 changed files with 605 additions and 160 deletions.
15 changes: 12 additions & 3 deletions dgraphtest/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,11 @@ type ClusterConfig struct {
volumes map[string]string
refillInterval time.Duration
uidLease int
portOffset int // exposed port offset for grpc/http port for both alpha/zero
bulkOutDir string
featureFlags []string
// exposed port offset for grpc/http port for both alpha/zero
portOffset int
bulkOutDir string
featureFlags []string
detectRace bool
}

func NewClusterConfig() ClusterConfig {
Expand All @@ -117,6 +119,7 @@ func NewClusterConfig() ClusterConfig {
refillInterval: 20 * time.Second,
uidLease: 50,
portOffset: -1,
detectRace: true,
}
}

Expand Down Expand Up @@ -192,3 +195,9 @@ func (cc ClusterConfig) WithNormalizeCompatibilityMode(mode string) ClusterConfi
cc.featureFlags = append(cc.featureFlags, fmt.Sprintf("normalize-compatibility-mode=%v", mode))
return cc
}

// Detect race in alphas
func (cc ClusterConfig) WithDetectRace() ClusterConfig {
cc.detectRace = true
return cc
}
25 changes: 25 additions & 0 deletions dgraphtest/local_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,10 @@ func (c *LocalCluster) Cleanup(verbose bool) {
ctx, cancel := context.WithTimeout(context.Background(), requestTimeout)
defer cancel()

if c.conf.detectRace {
c.DetectRaceInAlphas()
}

ro := types.ContainerRemoveOptions{RemoveVolumes: true, Force: true}
for _, aa := range c.alphas {
if err := c.dcli.ContainerRemove(ctx, aa.cid(), ro); err != nil {
Expand Down Expand Up @@ -889,3 +893,24 @@ func (c *LocalCluster) inspectContainer(containerID string) (string, error) {
}
return string(raw), nil
}

func (c *LocalCluster) DetectRaceInAlphas() bool {
for _, a := range c.alphas {
contLogs, err := c.getLogs(a.containerID)
if err != nil {
continue
}
if CheckIfRace([]byte(contLogs)) {
return true
}
}
return false
}

func CheckIfRace(output []byte) bool {
if strings.Contains(string(output), "WARNING: DATA RACE") {
log.Printf("[WARNING] DATA RACE DETECTED %s\n", string(output))
return true
}
return false
}
2 changes: 2 additions & 0 deletions dgraphtest/secrets/secret-key
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
��ϡ�
�}�^�6�ρ K��xԊ����s��%U�� ���aj|�Rl� XP��������E���u
4 changes: 2 additions & 2 deletions ee/acl/acl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2793,13 +2793,13 @@ func (asuite *AclTestSuite) TestDeleteGrootAndGuardiansUsingDelNQuadShouldFail()
// Try deleting groot user
_, err = gc.Mutate(mu)
require.Error(t, err, "Deleting groot user should have returned an error")
require.Contains(t, err.Error(), "Properties of guardians group and groot user cannot be deleted")
require.Contains(t, err.Error(), "properties of guardians group and groot user cannot be deleted")

mu = &api.Mutation{DelNquads: []byte(fmt.Sprintf("%s %s %s .", "<"+guardiansUid+">", "*", "*")), CommitNow: true}
// Try deleting guardians group
_, err = gc.Mutate(mu)
require.Error(t, err, "Deleting guardians group should have returned an error")
require.Contains(t, err.Error(), "Properties of guardians group and groot user cannot be deleted")
require.Contains(t, err.Error(), "properties of guardians group and groot user cannot be deleted")
}

func deleteGuardiansGroupAndGrootUserShouldFail(t *testing.T, hc *dgraphtest.HTTPClient) {
Expand Down
76 changes: 76 additions & 0 deletions systest/21million/bulk/integration_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
//go:build integration

/*
* Copyright 2023 Dgraph Labs, Inc. and Contributors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package bulk

import (
"os"
"path/filepath"
"testing"

"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"

"github.com/dgraph-io/dgraph/dgraphtest"
"github.com/dgraph-io/dgraph/testutil"
)

type BulkTestSuite struct {
suite.Suite
dc dgraphtest.Cluster
bulkDataDir string
}

func (bsuite *BulkTestSuite) SetupTest() {
bsuite.dc = dgraphtest.NewComposeCluster()
t := bsuite.T()
var err error
bsuite.bulkDataDir, err = os.MkdirTemp(os.TempDir(), "21millionBulk")
require.NoError(t, err)
require.NoError(t, downloadDataFiles(bsuite.bulkDataDir))
}

func (bsuite *BulkTestSuite) TearDownTest() {
testutil.DetectRaceInAlphas(testutil.DockerPrefix)
require.NoError(bsuite.T(), os.RemoveAll(bsuite.bulkDataDir))
}

func (bsuite *BulkTestSuite) Upgrade() {
// Not implemented for integration tests
}

func TestBulkTestSuite(t *testing.T) {
suite.Run(t, new(BulkTestSuite))
}

func (bsuite *BulkTestSuite) bulkLoader() error {
bulkDataDir := bsuite.bulkDataDir
rdfFile := filepath.Join(bulkDataDir, "21million.rdf.gz")
schemaFile := filepath.Join(bulkDataDir, "21million.schema")
require.NoError(bsuite.T(), testutil.MakeDirEmpty([]string{"out/0", "out/1", "out/2"}))
return testutil.BulkLoad(testutil.BulkOpts{
Zero: testutil.SockAddrZero,
Shards: 1,
RdfFile: rdfFile,
SchemaFile: schemaFile,
})
}

func (bsuite *BulkTestSuite) StartAlpha() error {
return testutil.StartAlphas("./alpha.yml")
}
134 changes: 104 additions & 30 deletions systest/21million/bulk/run_test.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//go:build integration
//go:build integration || upgrade

/*
* Copyright 2023 Dgraph Labs, Inc. and Contributors
Expand All @@ -19,47 +19,121 @@
package bulk

import (
"context"
"fmt"
"io"
"os"
"os/exec"
"path/filepath"
"testing"
"runtime"
"strings"
"time"

"github.com/dgraph-io/dgraph/systest/21million/common"
"github.com/dgraph-io/dgraph/testutil"
"github.com/pkg/errors"
"github.com/stretchr/testify/require"

"github.com/dgraph-io/dgraph/chunker"
"github.com/dgraph-io/dgraph/dgraphtest"
)

func TestQueries(t *testing.T) {
t.Run("Run queries", common.TestQueriesFor21Million)
var datafiles = map[string]string{
"1million-noindex.schema": "https://github.com/dgraph-io/benchmarks/blob/master/data/1million-noindex.schema?raw=true",
"1million.schema": "https://github.com/dgraph-io/benchmarks/blob/master/data/1million.schema?raw=true",
"1million.rdf.gz": "https://github.com/dgraph-io/benchmarks/blob/master/data/1million.rdf.gz?raw=true",
"21million.schema": "https://github.com/dgraph-io/benchmarks/blob/master/data/21million.schema?raw=true",
"21million.rdf.gz": "https://github.com/dgraph-io/benchmarks/blob/master/data/21million.rdf.gz?raw=true",
}

func TestMain(m *testing.M) {
schemaFile := filepath.Join(testutil.TestDataDirectory, "21million.schema")
rdfFile := filepath.Join(testutil.TestDataDirectory, "21million.rdf.gz")
if err := testutil.MakeDirEmpty([]string{"out/0", "out/1", "out/2"}); err != nil {
os.Exit(1)
}
// JSON output can be hundreds of lines and diffs can scroll off the terminal before you
// can look at them. This option allows saving the JSON to a specified directory instead
// for easier reviewing after the test completes.
//var savedir = flag.String("savedir", "",
// "directory to save json from test failures in")
//var quiet = flag.Bool("quiet", false,
// "just output whether json differs, not a diff")

if err := testutil.BulkLoad(testutil.BulkOpts{
Zero: testutil.SockAddrZero,
Shards: 1,
RdfFile: rdfFile,
SchemaFile: schemaFile,
}); err != nil {
cleanupAndExit(1)
}
func (bsuite *BulkTestSuite) TestQueriesFor21Million() {
t := bsuite.T()
_, thisFile, _, _ := runtime.Caller(0)
queryDir := filepath.Join(filepath.Dir(thisFile), "../queries")

if err := testutil.StartAlphas("./alpha.yml"); err != nil {
cleanupAndExit(1)
files, err := os.ReadDir(queryDir)
if err != nil {
t.Fatalf("Error reading directory: %s", err.Error())
}

exitCode := m.Run()
cleanupAndExit(exitCode)
//savepath := ""
//diffs := 0
for _, file := range files {
if !strings.HasPrefix(file.Name(), "query-") {
continue
}

bsuite.Run(file.Name(), func() {
require.NoError(t, bsuite.bulkLoader())

// start alphas
//require.NoError(t, c.Start())
require.NoError(t, bsuite.StartAlpha())

// Upgrade
bsuite.Upgrade()

// For this test we DON'T want to start with an empty database.
dg, cleanup, err := bsuite.dc.Client()
defer cleanup()
require.NoError(t, err)

filename := filepath.Join(queryDir, file.Name())
reader, cleanup := chunker.FileReader(filename, nil)
bytes, err := io.ReadAll(reader)
if err != nil {
t.Fatalf("Error reading file: %s", err.Error())
}
contents := string(bytes[:])
cleanup()

// The test query and expected result are separated by a delimiter.
bodies := strings.SplitN(contents, "\n---\n", 2)
// Dgraph can get into unhealthy state sometime. So, add retry for every query.
for retry := 0; retry < 3; retry++ {
// If a query takes too long to run, it probably means dgraph is stuck and there's
// no point in waiting longer or trying more tests.
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute)
resp, err := dg.NewTxn().Query(ctx, bodies[0])
cancel()

if retry < 2 && (err != nil || ctx.Err() == context.DeadlineExceeded) {
continue
}

if ctx.Err() == context.DeadlineExceeded {
t.Fatal("aborting test due to query timeout")
}

t.Logf("running %s", file.Name())
//if *savedir != "" {
// savepath = filepath.Join(*savedir, file.Name())
//}

dgraphtest.CompareJSON(bodies[1], string(resp.GetJson()))
}
})
}
//
//if *savedir != "" && diffs > 0 {
// t.Logf("test json saved in directory: %s", *savedir)
//}
}

func cleanupAndExit(exitCode int) {
if testutil.StopAlphasAndDetectRace([]string{"alpha1"}) {
// if there is race fail the test
exitCode = 1
func downloadDataFiles(testDir string) error {
for fname, link := range datafiles {
cmd := exec.Command("wget", "-O", fname, link)
cmd.Dir = testDir

if out, err := cmd.CombinedOutput(); err != nil {
return errors.Wrapf(err, fmt.Sprintf("error downloading a file: %s", string(out)))
}
}
_ = os.RemoveAll("out")
os.Exit(exitCode)
return nil
}
Loading

0 comments on commit c2e272b

Please sign in to comment.