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

Upgrade openfga to 1.2.0 #1010

Merged
merged 8 commits into from
Jul 26, 2023
Merged

Upgrade openfga to 1.2.0 #1010

merged 8 commits into from
Jul 26, 2023

Conversation

mina1460
Copy link
Contributor

@mina1460 mina1460 commented Jul 24, 2023

Description

Migrating Openfga version from 0.3.4 to 1.2.0

Fixes CSS-4799

Engineering checklist

Check only items that apply

  • Documentation updated
  • Covered by unit tests
  • Covered by integration tests

Signed-off-by: Mina Ashraf <[email protected]>
@mina1460 mina1460 changed the base branch from main to feature-rebac July 24, 2023 12:50
@mina1460 mina1460 marked this pull request as ready for review July 26, 2023 10:16
@@ -81,6 +82,13 @@ func getAuthModelDefinition() (_ []openfga.TypeDefinition, err error) {
return
}

var ok bool
schemaVersion, ok = wrapper["schema_version"].(string)
Copy link
Contributor

Choose a reason for hiding this comment

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

Thinking about this, we should also check that the OpenFGA charm does this too now.

internal/openfga/openfga_test.go Outdated Show resolved Hide resolved
.gitignore Outdated
@@ -18,5 +18,5 @@ local/vault/roleid.txt
*.crt
*.key
*.csr
jimmctl
/jimmctl
Copy link
Contributor

Choose a reason for hiding this comment

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

jimmctl is a binary no? Why /?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

leaving it as "jimmctl" without specifying that it is in the root level results in ignoring all paths that has jimmctl in it
This includes all files under the directory cmd/jimmctl

Copy link
Contributor

Choose a reason for hiding this comment

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

Ooooh, will this still ignore the binary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, this specifically ignores the binary and the binary only

@@ -305,9 +305,9 @@ func (s *accessControlSuite) TestAddRelation(c *gc.C) {
},
//Test group -> controller
{
input: tuple{"group-" + "test-group", "administrator", "controller-" + controller.UUID},
input: tuple{"group-" + "test-group#member", "administrator", "controller-" + controller.UUID},
Copy link
Contributor

Choose a reason for hiding this comment

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

Surprised this wasn't always like this (considering the comment above)

@@ -204,7 +204,7 @@ func (s *relationSuite) TestRemoveRelationViaFileSuperuser(c *gc.C) {
file, err := os.CreateTemp(".", "relations.json")
c.Assert(err, gc.IsNil)
defer os.Remove(file.Name())
testRelations := `[{"object":"group-` + group1 + `","relation":"member","target_object":"group-` + group3 + `"},{"object":"group-` + group2 + `","relation":"member","target_object":"group-` + group3 + `"}]`
testRelations := `[{"object":"group-` + group2 + `#member","relation":"member","target_object":"group-` + group3 + `"}]`
Copy link
Contributor

@ale8k ale8k Jul 26, 2023

Choose a reason for hiding this comment

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

How come we remove group 1*?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will get it back. Forgot it while debugging

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahh gotcha

@mina1460 mina1460 merged commit 54bf1c0 into canonical:feature-rebac Jul 26, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants