Skip to content

Commit

Permalink
fix(Export/Import): Live load exported schema (#5663) (#5696)
Browse files Browse the repository at this point in the history
This PR fixes the issue where live loader was not able to import the schema exported using export request. It does so, by allowing the alter schema request for pre-defined types to go through if the type in the request is same as the initial internal type.

This issue was introduced as a result of #5185.

This PR also disallows a pre-defined type from being dropped.

Fixes #DGRAPH-1694.

(cherry picked from commit 76ae60b)
  • Loading branch information
abhimanyusinghgaur authored Jun 19, 2020
1 parent 64a9438 commit 9f690df
Show file tree
Hide file tree
Showing 6 changed files with 214 additions and 31 deletions.
59 changes: 59 additions & 0 deletions dgraph/cmd/live/load-uids/load_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"path"
"runtime"
"testing"
"time"

"github.com/dgraph-io/dgo/v200"
"github.com/dgraph-io/dgo/v200/protos/api"
Expand All @@ -41,6 +42,12 @@ var (
dg *dgo.Dgraph
)

const (
alphaName = "alpha1"
alphaExportPath = alphaName + ":/data/" + alphaName + "/export"
localExportPath = "./export_copy"
)

func checkDifferentUid(t *testing.T, wantMap, gotMap map[string]interface{}) {
require.NotEqual(t, gotMap["q"].([]interface{})[0].(map[string]interface{})["uid"],
wantMap["q"].([]interface{})[0].(map[string]interface{})["uid"],
Expand Down Expand Up @@ -171,6 +178,58 @@ func TestLiveLoadRdfUidDiscard(t *testing.T) {
checkLoadedData(t, true)
}

func TestLiveLoadExportedSchema(t *testing.T) {
testutil.DropAll(t, dg)

// initiate export
params := &testutil.GraphQLParams{
Query: `
mutation {
export(input: {format: "rdf"}) {
response {
code
message
}
}
}`,
}
accessJwt, _ := testutil.GrootHttpLogin("http://" + testutil.SockAddrHttp + "/admin")
resp := testutil.MakeGQLRequestWithAccessJwt(t, params, accessJwt)
require.Nilf(t, resp.Errors, resp.Errors.Error())

// wait a bit to be sure export is complete
time.Sleep(time.Second)

// copy the export files from docker
exportId := copyExportToLocalFs(t)

// then loading the exported files should work
pipeline := [][]string{
{testutil.DgraphBinaryPath(), "live",
"--schema", localExportPath + "/" + exportId + "/g01.schema.gz",
"--files", localExportPath + "/" + exportId + "/g01.rdf.gz",
"--encryption_key_file", testDataDir + "/../../../../ee/enc/test-fixtures/enc-key",
"--alpha", alphaService, "--zero", zeroService, "-u", "groot", "-p", "password"},
}
err := testutil.Pipeline(pipeline)
require.NoError(t, err, "live loading exported schema exited with error")

// cleanup copied export files
require.NoError(t, os.RemoveAll(localExportPath), "Error removing export copy directory")
}

func copyExportToLocalFs(t *testing.T) string {
require.NoError(t, os.RemoveAll(localExportPath), "Error removing directory")
require.NoError(t, testutil.DockerCp(alphaExportPath, localExportPath),
"Error copying files from docker container")

childDirs, err := ioutil.ReadDir(localExportPath)
require.NoError(t, err, "Couldn't read local export copy directory")
require.True(t, len(childDirs) > 0, "Local export copy directory is empty!!!")

return childDirs[0].Name()
}

func TestMain(m *testing.M) {
_, thisFile, _, _ := runtime.Caller(0)
testDataDir = path.Dir(thisFile)
Expand Down
1 change: 1 addition & 0 deletions dgraph/cmd/live/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ func init() {
Run: func(cmd *cobra.Command, args []string) {
defer x.StartProfile(Live.Conf).Stop()
if err := run(); err != nil {
x.Check2(fmt.Fprintf(os.Stderr, "%s", err.Error()))
os.Exit(1)
}
},
Expand Down
34 changes: 23 additions & 11 deletions edgraph/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,11 +207,10 @@ func (s *Server) Alter(ctx context.Context, op *api.Operation) (*api.Payload, er
attr = op.DropValue
}

// Reserved predicates cannot be dropped.
if x.IsReservedPredicate(attr) {
err := errors.Errorf("predicate %s is reserved and is not allowed to be dropped",
attr)
return empty, err
// Pre-defined predicates cannot be dropped.
if x.IsPreDefinedPredicate(attr) {
return empty, errors.Errorf("predicate %s is pre-defined and is not allowed to be"+
" dropped", attr)
}

nq := &api.NQuad{
Expand All @@ -235,6 +234,12 @@ func (s *Server) Alter(ctx context.Context, op *api.Operation) (*api.Payload, er
return empty, errors.Errorf("If DropOp is set to TYPE, DropValue must not be empty")
}

// Pre-defined types cannot be dropped.
if x.IsPreDefinedType(op.DropValue) {
return empty, errors.Errorf("type %s is pre-defined and is not allowed to be dropped",
op.DropValue)
}

m.DropOp = pb.Mutations_TYPE
m.DropValue = op.DropValue
_, err := query.ApplyMutations(ctx, m)
Expand All @@ -254,10 +259,9 @@ func (s *Server) Alter(ctx context.Context, op *api.Operation) (*api.Payload, er
for _, update := range result.Preds {
// Pre-defined predicates cannot be altered but let the update go through
// if the update is equal to the existing one.
if schema.IsPreDefinedPredicateChanged(update.Predicate, update) {
err := errors.Errorf("predicate %s is reserved and is not allowed to be modified",
update.Predicate)
return nil, err
if schema.IsPreDefPredChanged(update) {
return nil, errors.Errorf("predicate %s is pre-defined and is not allowed to be"+
" modified", update.Predicate)
}

if err := validatePredName(update.Predicate); err != nil {
Expand All @@ -276,8 +280,16 @@ func (s *Server) Alter(ctx context.Context, op *api.Operation) (*api.Payload, er
}

for _, typ := range result.Types {
// Users are not allowed to create types in reserved namespace.
if x.IsReservedType(typ.TypeName) {
// Pre-defined types cannot be altered but let the update go through
// if the update is equal to the existing one.
if schema.IsPreDefTypeChanged(typ) {
return nil, errors.Errorf("type %s is pre-defined and is not allowed to be modified",
typ.TypeName)
}

// Users are not allowed to create types in reserved namespace. But, there are pre-defined
// types for which the update should go through if the update is equal to the existing one.
if x.IsReservedType(typ.TypeName) && !x.IsPreDefinedType(typ.TypeName) {
return nil, errors.Errorf("Can't alter type `%s` as it is prefixed with `dgraph.` "+
"which is reserved as the namespace for dgraph's internal types/predicates.",
typ.TypeName)
Expand Down
67 changes: 55 additions & 12 deletions ee/acl/acl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,14 +223,22 @@ func resetUser(t *testing.T) {
glog.Infof("created user")
}

func TestReservedPredicates(t *testing.T) {
// This test uses the groot account to ensure that reserved predicates
func TestPreDefinedPredicates(t *testing.T) {
// This test uses the groot account to ensure that pre-defined predicates
// cannot be altered even if the permissions allow it.
dg1, err := testutil.DgraphClientWithGroot(testutil.SockAddr)
if err != nil {
t.Fatalf("Error while getting a dgraph client: %v", err)
}
alterReservedPredicates(t, dg1)
require.NoError(t, err, "Error while getting a dgraph client")

alterPreDefinedPredicates(t, dg1)
}

func TestPreDefinedTypes(t *testing.T) {
// This test uses the groot account to ensure that pre-defined types
// cannot be altered even if the permissions allow it.
dg, err := testutil.DgraphClientWithGroot(testutil.SockAddr)
require.NoError(t, err, "Error while getting a dgraph client")

alterPreDefinedTypes(t, dg)
}

func TestAuthorization(t *testing.T) {
Expand Down Expand Up @@ -310,11 +318,11 @@ var query = fmt.Sprintf(`
}`, predicateToRead, queryAttr)
var schemaQuery = "schema {}"

func alterReservedPredicates(t *testing.T, dg *dgo.Dgraph) {
func alterPreDefinedPredicates(t *testing.T, dg *dgo.Dgraph) {
ctx := context.Background()

// Test that alter requests are allowed if the new update is the same as
// the initial update for a reserved predicate.
// the initial update for a pre-defined predicate.
err := dg.Alter(ctx, &api.Operation{
Schema: "dgraph.xid: string @index(exact) @upsert .",
})
Expand All @@ -325,22 +333,57 @@ func alterReservedPredicates(t *testing.T, dg *dgo.Dgraph) {
})
require.Error(t, err)
require.Contains(t, err.Error(),
"predicate dgraph.xid is reserved and is not allowed to be modified")
"predicate dgraph.xid is pre-defined and is not allowed to be modified")

err = dg.Alter(ctx, &api.Operation{
DropAttr: "dgraph.xid",
})
require.Error(t, err)
require.Contains(t, err.Error(),
"predicate dgraph.xid is reserved and is not allowed to be dropped")
"predicate dgraph.xid is pre-defined and is not allowed to be dropped")

// Test that reserved predicates act as case-insensitive.
// Test that pre-defined predicates act as case-insensitive.
err = dg.Alter(ctx, &api.Operation{
Schema: "dgraph.XID: int .",
})
require.Error(t, err)
require.Contains(t, err.Error(),
"predicate dgraph.XID is reserved and is not allowed to be modified")
"predicate dgraph.XID is pre-defined and is not allowed to be modified")
}

func alterPreDefinedTypes(t *testing.T, dg *dgo.Dgraph) {
ctx := context.Background()

// Test that alter requests are allowed if the new update is the same as
// the initial update for a pre-defined type.
err := dg.Alter(ctx, &api.Operation{
Schema: `
type dgraph.type.Group {
dgraph.xid
dgraph.acl.rule
}
`,
})
require.NoError(t, err)

err = dg.Alter(ctx, &api.Operation{
Schema: `
type dgraph.type.Group {
dgraph.xid
}
`,
})
require.Error(t, err)
require.Contains(t, err.Error(),
"type dgraph.type.Group is pre-defined and is not allowed to be modified")

err = dg.Alter(ctx, &api.Operation{
DropOp: api.Operation_TYPE,
DropValue: "dgraph.type.Group",
})
require.Error(t, err)
require.Contains(t, err.Error(),
"type dgraph.type.Group is pre-defined and is not allowed to be dropped")
}

func queryPredicateWithUserAccount(t *testing.T, dg *dgo.Dgraph, shouldFail bool) {
Expand Down
59 changes: 51 additions & 8 deletions schema/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -533,10 +533,25 @@ func LoadTypesFromDb() error {
return nil
}

// InitialTypes returns the schema updates to insert at the begining of
// Dgraph's execution. It looks at the schema state to determine which
// InitialTypes returns the type updates to insert at the beginning of
// Dgraph's execution. It looks at the worker options to determine which
// types to insert.
func InitialTypes() []*pb.TypeUpdate {
return initialTypesInternal(false)
}

// CompleteInitialTypes returns all the type updates regardless of the worker
// options. This is useful in situations where the worker options are not known
// in advance or it is required to consider all initial pre-defined types. An
// example of such situation is while allowing type updates to go through during
// alter if they are same as existing pre-defined types. This is useful for
// live loading a previously exported schema.
func CompleteInitialTypes() []*pb.TypeUpdate {
return initialTypesInternal(true)
}

// NOTE: whenever defining a new type here, please also add it in x/keys.go: preDefinedTypeMap
func initialTypesInternal(all bool) []*pb.TypeUpdate {
var initialTypes []*pb.TypeUpdate
initialTypes = append(initialTypes,
&pb.TypeUpdate{
Expand All @@ -553,7 +568,7 @@ func InitialTypes() []*pb.TypeUpdate {
},
})

if x.WorkerConfig.AclEnabled {
if all || x.WorkerConfig.AclEnabled {
// These type definitions are required for deleteUser and deleteGroup GraphQL API to work
// properly.
initialTypes = append(initialTypes, &pb.TypeUpdate{
Expand Down Expand Up @@ -682,24 +697,52 @@ func initialSchemaInternal(all bool) []*pb.SchemaUpdate {
return initialSchema
}

// IsPreDefinedPredicateChanged returns true if the initial update for the pre-defined
// predicate pred is different than the passed update.
func IsPreDefinedPredicateChanged(pred string, update *pb.SchemaUpdate) bool {
// IsPreDefPredChanged returns true if the initial update for the pre-defined
// predicate is different than the passed update.
//If the passed update is not a pre-defined predicate then it just returns false.
func IsPreDefPredChanged(update *pb.SchemaUpdate) bool {
// Return false for non-pre-defined predicates.
if !x.IsPreDefinedPredicate(pred) {
if !x.IsPreDefinedPredicate(update.Predicate) {
return false
}

initialSchema := CompleteInitialSchema()
for _, original := range initialSchema {
if original.Predicate != pred {
if original.Predicate != update.Predicate {
continue
}
return !proto.Equal(original, update)
}
return true
}

// IsPreDefTypeChanged returns true if the initial update for the pre-defined
// type is different than the passed update.
// If the passed update is not a pre-defined type than it just returns false.
func IsPreDefTypeChanged(update *pb.TypeUpdate) bool {
// Return false for non-pre-defined types.
if !x.IsPreDefinedType(update.TypeName) {
return false
}

initialTypes := CompleteInitialTypes()
for _, original := range initialTypes {
if original.TypeName != update.TypeName {
continue
}
if len(original.Fields) != len(update.Fields) {
return true
}
for i, field := range original.Fields {
if field.Predicate != update.Fields[i].Predicate {
return true
}
}
}

return false
}

func reset() {
pstate = new(state)
pstate.init()
Expand Down
25 changes: 25 additions & 0 deletions x/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,13 @@ var internalPredicateMap = map[string]struct{}{
"uid": {},
}

var preDefinedTypeMap = map[string]struct{}{
"dgraph.graphql": {},
"dgraph.type.User": {},
"dgraph.type.Group": {},
"dgraph.type.Rule": {},
}

// IsGraphqlReservedPredicate returns true if it is the predicate is reserved by graphql.
func IsGraphqlReservedPredicate(pred string) bool {
_, ok := graphqlReservedPredicate[pred]
Expand Down Expand Up @@ -630,10 +637,28 @@ func IsInternalPredicate(pred string) bool {
// We reserve `dgraph.` as the namespace for the types/predicates we may create in future.
// So, users are not allowed to create a type under this namespace.
// Hence, we should always define internal types under `dgraph.` namespace.
//
// Pre-defined types are subset of reserved types.
//
// When critical, use IsPreDefinedType(typ string) to find out whether the typ was
// actually defined internally or not.
func IsReservedType(typ string) bool {
return isReservedName(typ)
}

// IsPreDefinedType returns true only if the typ has been defined by dgraph internally.
// For example, `dgraph.graphql` or ACL types are defined in the initial internal types.
//
// We reserve `dgraph.` as the namespace for the types/predicates we may create in future.
// So, users are not allowed to create a predicate under this namespace.
// Hence, we should always define internal types under `dgraph.` namespace.
//
// Pre-defined types are subset of reserved types.
func IsPreDefinedType(typ string) bool {
_, ok := preDefinedTypeMap[typ]
return ok
}

// isReservedName returns true if the given name is prefixed with `dgraph.`
func isReservedName(name string) bool {
return strings.HasPrefix(strings.ToLower(name), "dgraph.")
Expand Down

0 comments on commit 9f690df

Please sign in to comment.