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.

Sign up for GitHub

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

feat: Add node identity #3125

Merged
merged 39 commits into from
Oct 25, 2024
Merged

feat: Add node identity #3125

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
7caedf3
Remove code duplication
islamaliev Oct 8, 2024
a1ea081
Assign identity to a node
islamaliev Oct 8, 2024
f916707
WIP
islamaliev Oct 8, 2024
d916c33
Return RawIdentity, add test
islamaliev Oct 10, 2024
1238aba
Fix lint
islamaliev Oct 10, 2024
1b92c70
Update docs
islamaliev Oct 10, 2024
61c8fb0
Update mocks
islamaliev Oct 10, 2024
82dc3b2
Minor refactor
islamaliev Oct 10, 2024
30a028f
PR fixup
islamaliev Oct 12, 2024
2bd361a
Polish
islamaliev Oct 12, 2024
940177d
Update mocks
islamaliev Oct 12, 2024
55413a3
PR fixup
islamaliev Oct 15, 2024
5ecf6ab
Polish
islamaliev Oct 15, 2024
a32a1f4
PR fixup
islamaliev Oct 15, 2024
62e38c5
PR fixup
islamaliev Oct 15, 2024
ed20e57
PR fixup
islamaliev Oct 17, 2024
a3396bc
Update docs
islamaliev Oct 17, 2024
3f03aa5
Rename command to node-identity
islamaliev Oct 18, 2024
5b2f935
Add assign-node-identity command
islamaliev Oct 20, 2024
b9ebd23
Update docs
islamaliev Oct 20, 2024
d158f83
Lint fix
islamaliev Oct 20, 2024
ab5dc33
Update mocks
islamaliev Oct 20, 2024
4c73fb4
Create parent command node-identity
islamaliev Oct 20, 2024
5946638
PR fixup
islamaliev Oct 21, 2024
8e39ec3
Merge remote-tracking branch 'upstream/develop' into feat/node-identity
islamaliev Oct 21, 2024
0530ab7
Make identity token updatable
islamaliev Oct 22, 2024
ab3a9ea
Update docs
islamaliev Oct 22, 2024
0e0a252
Fix lint
islamaliev Oct 22, 2024
5b3a5c4
Merge remote-tracking branch 'upstream/develop' into feat/node-identity
islamaliev Oct 22, 2024
a0f173f
Turn 2d array of identities into 1d (WIP)
islamaliev Oct 12, 2024
b6d148b
Add clear distinction between user and node identity
islamaliev Oct 13, 2024
95fc645
Pass ctx explicitly
islamaliev Oct 22, 2024
b154dbe
Remove duration from node's identity
islamaliev Oct 23, 2024
af4e2f9
Remove node-identity assign command
islamaliev Oct 23, 2024
2869d87
Polish
islamaliev Oct 24, 2024
3210fbf
Merge remote-tracking branch 'upstream/develop' into feat/node-identity
islamaliev Oct 24, 2024
e4fc548
Make identityRef optional
islamaliev Oct 25, 2024
7fc8e3d
Rename UserIdentity to ClientIdentity
islamaliev Oct 25, 2024
f3ca301
Merge remote-tracking branch 'upstream/develop' into feat/node-identity
islamaliev Oct 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions tests/integration/acp.go
View file
Open in desktop
islamaliev marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func addPolicyACP(

nodeIDs, nodes := getNodesWithIDs(action.NodeID, s.nodes)
for index, node := range nodes {
ctx := getContextWithIdentity(s, action.Identity, nodeIDs[index])
ctx := getContextWithIdentity(s.ctx, s, action.Identity, nodeIDs[index])
Copy link
Member

Choose a reason for hiding this comment

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

question: Curious why this was done?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what exactly? We need a context with an identity in it for sending a request.
Please clarify your question. I also documented some of my decisions. Let me know, if they are not clear enough.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I will clarify. I am wondering why the function couldn't access s.ctx when the function is consuming the s already in the function argument. Why did we need to have a new parameter and pass s.ctx separately?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because in some parts of the code the context is not stored in s.ctx. I don't know if there is a good reason for this.
I can actually change it to something like this:

s.ctx = newCtx
updateContextWithIdentity(s, action.Identity, nodeIDs[index]) // will store in s.ctx
policyResult, err := node.AddPolicy(s.ctx, action.Policy)

Not sure if that makes it better, but I don't have a strong opinion here.

Copy link
Member

Choose a reason for hiding this comment

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

because in some parts of the code the context is not stored in s.ctx. I don't know if there is a good reason for this. I can actually change it to something like this:

s.ctx = newCtx
updateContextWithIdentity(s, action.Identity, nodeIDs[index]) // will store in s.ctx
policyResult, err := node.AddPolicy(s.ctx, action.Policy)

Not sure if that makes it better, but I don't have a strong opinion here.

Oh wow, perhaps worth investigating later where and why that happens. I am happy with the suggested solution you gave for places that have missing ctx

policyResult, err := node.AddPolicy(ctx, action.Policy)

expectedErrorRaised := AssertError(s.t, s.testCase.Description, err, action.ExpectedError)
Expand Down Expand Up @@ -188,7 +188,7 @@ func addDocActorRelationshipACP(
collectionName, docID := getCollectionAndDocInfo(s, action.CollectionID, action.DocID, nodeID)

exists, err := node.AddDocActorRelationship(
getContextWithIdentity(s, action.RequestorIdentity, nodeID),
getContextWithIdentity(s.ctx, s, action.RequestorIdentity, nodeID),
collectionName,
docID,
action.Relation,
Expand Down Expand Up @@ -269,7 +269,7 @@ func deleteDocActorRelationshipACP(
collectionName, docID := getCollectionAndDocInfo(s, action.CollectionID, action.DocID, nodeID)

deleteDocActorRelationshipResult, err := node.DeleteDocActorRelationship(
getContextWithIdentity(s, action.RequestorIdentity, nodeID),
getContextWithIdentity(s.ctx, s, action.RequestorIdentity, nodeID),
collectionName,
docID,
action.Relation,
Expand Down
11 changes: 6 additions & 5 deletions tests/integration/identity.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,11 @@ import (
"math/rand"

"github.com/decred/dcrd/dcrec/secp256k1/v4"
acpIdentity "github.com/sourcenetwork/defradb/acp/identity"
identity "github.com/sourcenetwork/defradb/acp/identity"
"github.com/sourcenetwork/immutable"
"github.com/stretchr/testify/require"

acpIdentity "github.com/sourcenetwork/defradb/acp/identity"
identity "github.com/sourcenetwork/defradb/acp/identity"
islamaliev marked this conversation as resolved.
Show resolved Hide resolved
)

// identRef is a type that refers to a specific identity of a certain type.
Expand Down Expand Up @@ -128,12 +129,12 @@ func generateIdentity(s *state) acpIdentity.Identity {
// getContextWithIdentity returns a context with the identity for the given reference and node index.
// If the identity does not exist, it will be generated.
// The identity added to the context is prepared for a request, i.e. its [Identity.BearerToken] is set.
func getContextWithIdentity(s *state, ref identRef, nodeIndex int) context.Context {
func getContextWithIdentity(ctx context.Context, s *state, ref identRef, nodeIndex int) context.Context {
if !ref.hasValue {
return s.ctx
return ctx
}
ident := getIdentityForRequest(s, ref, nodeIndex)
return identity.WithContext(s.ctx, immutable.Some(ident))
return identity.WithContext(ctx, immutable.Some(ident))
}

func getIdentityDID(s *state, ident identRef) string {
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/state.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ type state struct {
txns []datastore.Txn

// identities contains all identities created in this test.
// The map key is the identity reference that uniquely identifies identities of different
// The map key is the identity reference that uniquely identifies identities of different
// types. See [identRef].
// The map value is the identity holder that contains the identity itself and token
// generated for different target nodes. See [identityHolder].
Expand Down
18 changes: 8 additions & 10 deletions tests/integration/utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1313,7 +1313,7 @@ func createDocViaColSave(
}

func makeContextForDocCreate(s *state, ctx context.Context, nodeIndex int, action *CreateDoc) context.Context {
ctx = getContextWithIdentity(s, action.Identity, nodeIndex)
ctx = getContextWithIdentity(ctx, s, action.Identity, nodeIndex)
ctx = encryption.SetContextConfigFromParams(ctx, action.IsDocEncrypted, action.EncryptedFields)
return ctx
}
Expand Down Expand Up @@ -1392,8 +1392,7 @@ func createDocViaGQL(
req := fmt.Sprintf(`mutation { %s(%s) { _docID } }`, key, params)

txn := getTransaction(s, node, immutable.None[int](), action.ExpectedError)
s.ctx = db.SetContextTxn(s.ctx, txn)
ctx := getContextWithIdentity(s, action.Identity, nodeIndex)
ctx := getContextWithIdentity(db.SetContextTxn(s.ctx, txn), s, action.Identity, nodeIndex)

result := node.ExecRequest(ctx, req)
if len(result.GQL.Errors) > 0 {
Expand Down Expand Up @@ -1447,7 +1446,7 @@ func deleteDoc(
for index, node := range nodes {
nodeID := nodeIDs[index]
collection := s.collections[nodeID][action.CollectionID]
ctx := getContextWithIdentity(s, action.Identity, nodeID)
ctx := getContextWithIdentity(s.ctx, s, action.Identity, nodeID)
err := withRetryOnNode(
node,
func() error {
Expand Down Expand Up @@ -1520,7 +1519,7 @@ func updateDocViaColSave(
nodeIndex int,
collection client.Collection,
) error {
ctx := getContextWithIdentity(s, action.Identity, nodeIndex)
ctx := getContextWithIdentity(s.ctx, s, action.Identity, nodeIndex)

doc, err := collection.Get(ctx, s.docIDs[action.CollectionID][action.DocID], true)
if err != nil {
Expand All @@ -1540,7 +1539,7 @@ func updateDocViaColUpdate(
nodeIndex int,
collection client.Collection,
) error {
ctx := getContextWithIdentity(s, action.Identity, nodeIndex)
ctx := getContextWithIdentity(s.ctx, s, action.Identity, nodeIndex)

doc, err := collection.Get(ctx, s.docIDs[action.CollectionID][action.DocID], true)
if err != nil {
Expand Down Expand Up @@ -1576,7 +1575,7 @@ func updateDocViaGQL(
input,
)

ctx := getContextWithIdentity(s, action.Identity, nodeIndex)
ctx := getContextWithIdentity(s.ctx, s, action.Identity, nodeIndex)

result := node.ExecRequest(ctx, request)
if len(result.GQL.Errors) > 0 {
Expand All @@ -1594,7 +1593,7 @@ func updateWithFilter(s *state, action UpdateWithFilter) {
for index, node := range nodes {
nodeID := nodeIDs[index]
collection := s.collections[nodeID][action.CollectionID]
ctx := getContextWithIdentity(s, action.Identity, nodeID)
ctx := getContextWithIdentity(s.ctx, s, action.Identity, nodeID)
err := withRetryOnNode(
node,
func() error {
Expand Down Expand Up @@ -1835,8 +1834,7 @@ func executeRequest(
nodeID := nodeIDs[index]
txn := getTransaction(s, node, action.TransactionID, action.ExpectedError)

s.ctx = db.SetContextTxn(s.ctx, txn)
ctx := getContextWithIdentity(s, action.Identity, nodeID)
ctx := getContextWithIdentity(db.SetContextTxn(s.ctx, txn), s, action.Identity, nodeID)

var options []client.RequestOption
if action.OperationName.HasValue() {
Expand Down
Loading