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

feat(nodebuilder/node): Implement Module #1313

Merged
merged 12 commits into from
Dec 13, 2022

Conversation

renaynay
Copy link
Member

@renaynay renaynay commented Nov 2, 2022

Resolves #977

@codecov-commenter
Copy link

codecov-commenter commented Nov 19, 2022

Codecov Report

Merging #1313 (f4ae5a0) into main (52048ce) will decrease coverage by 0.20%.
The diff coverage is 22.53%.

@@            Coverage Diff             @@
##             main    #1313      +/-   ##
==========================================
- Coverage   55.37%   55.17%   -0.21%     
==========================================
  Files         199      203       +4     
  Lines       12053    12124      +71     
==========================================
+ Hits         6674     6689      +15     
- Misses       4714     4770      +56     
  Partials      665      665              
Impacted Files Coverage Δ
nodebuilder/module.go 100.00% <ø> (ø)
nodebuilder/node/node.go 0.00% <0.00%> (ø)
nodebuilder/node/mocks/api.go 10.00% <10.00%> (ø)
nodebuilder/node/admin.go 26.66% <26.66%> (ø)
api/rpc/client/client.go 79.31% <100.00%> (+0.73%) ⬆️
nodebuilder/node/module.go 100.00% <100.00%> (ø)
nodebuilder/rpc/constructors.go 100.00% <100.00%> (ø)
das/coordinator.go 86.76% <0.00%> (-1.48%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@renaynay renaynay force-pushed the impl-node-mod branch 3 times, most recently from 82e27c9 to 8b1f05d Compare November 25, 2022 12:24
@renaynay renaynay marked this pull request as ready for review November 25, 2022 12:54
@renaynay renaynay marked this pull request as draft November 25, 2022 12:56
nodebuilder/node/node.go Outdated Show resolved Hide resolved
nodebuilder/node/node.go Outdated Show resolved Hide resolved
distractedm1nd
distractedm1nd previously approved these changes Dec 7, 2022
nodebuilder/node/admin.go Outdated Show resolved Hide resolved
docs/adr/adr-009-public-api.md Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
docs/adr/adr-009-public-api.md Outdated Show resolved Hide resolved
nodebuilder/node/node.go Outdated Show resolved Hide resolved
distractedm1nd
distractedm1nd previously approved these changes Dec 8, 2022
nodebuilder/node/admin.go Outdated Show resolved Hide resolved
refactor(nodebuilder/node): add authverify and authnew methods
feat(nodebuilder/node): Add mocks and integrate node module
…favour of AdminInfo method that returns node type and api version, do not return build info
docs/adr/adr-009-public-api.md Outdated Show resolved Hide resolved
Copy link
Member

@Wondertan Wondertan left a comment

Choose a reason for hiding this comment

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

LGTM modulo two comments

docs/adr/adr-009-public-api.md Outdated Show resolved Hide resolved
nodebuilder/node/admin.go Outdated Show resolved Hide resolved
Copy link
Member

@Wondertan Wondertan left a comment

Choose a reason for hiding this comment

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

🚢

@renaynay renaynay merged commit cf7ad84 into celestiaorg:main Dec 13, 2022
@renaynay renaynay deleted the impl-node-mod branch December 13, 2022 11:13
renaynay added a commit that referenced this pull request Dec 20, 2022
Related to #1187 

**This PR is based on #1313** 

**It implements:** 
* providing a new JWT secret from the `nodebuilder/node` module to both
the node module and the rpc server.
* auth middleware for the `rpc.Server` that verifies the given token
with the server's secret

In a follow-up PR will come the ability to create an rpc client with
elevated permissions (this branch restricts client access to read-only
API methods).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:api Related to celestia-node API area:node Node area:rpc kind:feat Attached to feature PRs
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Implement additional modules that don't yet exist
5 participants