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.

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 tool to upgrade ACLs #5016

Merged
merged 7 commits into from
Mar 30, 2020
Merged

Add tool to upgrade ACLs #5016

merged 7 commits into from
Mar 30, 2020

Conversation

mangalaman93
Copy link
Member

@mangalaman93 mangalaman93 commented Mar 24, 2020

TODO

  • Add docs
  • Test the tool

This change is Reviewable

Docs Preview: Dgraph Preview

@mangalaman93 mangalaman93 requested review from manishrjain and a team as code owners March 24, 2020 16:34
@mangalaman93 mangalaman93 force-pushed the animesh/upgrade branch 2 times, most recently from a8bf7cb to f79436f Compare March 30, 2020 09:10
Copy link
Contributor

@manishrjain manishrjain left a comment

Choose a reason for hiding this comment

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

:lgtm: Some comments.

Reviewed 2 of 2 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @mangalaman93)


contrib/aclupgrade/upgrade.go, line 1 at r1 (raw file):

package main

Add license.

Also, move it to a Dgraph subcommand called 'upgrade'. And inside the main, just have a flag for acl upgrade for now.

dgraph upgrade --acls -a "localhost:8080"


contrib/aclupgrade/upgrade.go, line 42 at r1 (raw file):

	userName := flag.String("u", "", "Username to login to Dgraph cluster")
	password := flag.String("p", "", "Password to login to Dgraph cluster")
	deleteOld := flag.Bool("d", false, "Delete the older ACL predicates")

Maybe we should delete by default. Otherwise, a re-run of this script would end up duplicating the rules. Do the deletion only after successful mutation.

Copy link
Member Author

@mangalaman93 mangalaman93 left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @manishrjain)


contrib/aclupgrade/upgrade.go, line 1 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Add license.

Also, move it to a Dgraph subcommand called 'upgrade'. And inside the main, just have a flag for acl upgrade for now.

dgraph upgrade --acls -a "localhost:8080"

Done.


contrib/aclupgrade/upgrade.go, line 42 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Maybe we should delete by default. Otherwise, a re-run of this script would end up duplicating the rules. Do the deletion only after successful mutation.

Done.

upgrade/upgrade.go Outdated Show resolved Hide resolved
Copy link
Contributor

@manishrjain manishrjain left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 3 of 4 files at r2, 1 of 1 files at r3.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @danielmai, @mangalaman93, @manishrjain, and @MichaelJCompton)

@mangalaman93 mangalaman93 merged commit 2c93bc8 into master Mar 30, 2020
@mangalaman93 mangalaman93 deleted the animesh/upgrade branch March 30, 2020 20:45
danielmai pushed a commit that referenced this pull request Mar 30, 2020
danielmai added a commit that referenced this pull request Mar 31, 2020
Add dgraph upgrade tool to upgrade the ACL data format.

Co-authored-by: Animesh Chandra Pathak <[email protected]>
Co-authored-by: Aman Mangal <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants