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

Jump to bottom

feat(amphora-service)!: add opa client and service #74

Merged
merged 8 commits into from
Dec 17, 2024
Merged

feat(amphora-service)!: add opa client and service #74

merged 8 commits into from
Dec 17, 2024

Conversation

sbckr
Copy link
Member

@sbckr sbckr commented Nov 14, 2024

No description provided.

@sbckr sbckr changed the title Authorization feat(amphora-service)!: add opa client and service Nov 14, 2024
@codecov Codecov
Copy link

codecov bot commented Nov 14, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 97.08738% with 6 lines in your changes missing coverage. Please review.

Project coverage is 90.15%. Comparing base (4a77d30) to head (f113daa).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
...io/carbynestack/amphora/service/opa/JwtReader.java 88.00% 2 Missing and 1 partial ⚠️
...a/service/rest/RestControllerExceptionHandler.java 0.00% 3 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master      #74      +/-   ##
============================================
+ Coverage     90.04%   90.15%   +0.10%     
- Complexity      348      397      +49     
============================================
  Files            49       59      +10     
  Lines          1326     1483     +157     
  Branches         70       78       +8     
============================================
+ Hits           1194     1337     +143     
- Misses           80       93      +13     
- Partials         52       53       +1
Flag Coverage Δ
service 93.14% <97.08%> (-0.47%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...carbynestack/amphora/service/config/OpaConfig.java 100.00% <100.00%> (ø)
...ack/amphora/service/exceptions/CsOpaException.java 100.00% <100.00%> (ø)
...hora/service/exceptions/UnauthorizedException.java 100.00% <100.00%> (ø)
...io/carbynestack/amphora/service/opa/OpaClient.java 100.00% <100.00%> (ø)
...ynestack/amphora/service/opa/OpaClientRequest.java 100.00% <100.00%> (ø)
...o/carbynestack/amphora/service/opa/OpaRequest.java 100.00% <100.00%> (ø)
...rbynestack/amphora/service/opa/OpaRequestBody.java 100.00% <100.00%> (ø)
...io/carbynestack/amphora/service/opa/OpaResult.java 100.00% <100.00%> (ø)
...o/carbynestack/amphora/service/opa/OpaService.java 100.00% <100.00%> (ø)
...a/service/persistence/metadata/StorageService.java 96.42% <100.00%> (-2.18%) ⬇️
... and 6 more

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4a77d30...f113daa. Read the comment docs.

@sbckr sbckr marked this pull request as ready for review November 18, 2024 14:54
@sbckr sbckr requested a review from a team as a code owner November 18, 2024 14:54
@sbckr sbckr requested a review from strieflin November 18, 2024 14:54
sbckr
sbckr commented Dec 16, 2024
View reviewed changes
Copy link
Member Author

@sbckr sbckr left a comment

Choose a reason for hiding this comment

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

see comments

amphora-service/src/main/java/io/carbynestack/amphora/service/calculation/SecretShareUtil.java Outdated Show resolved Hide resolved
amphora-service/src/main/java/io/carbynestack/amphora/service/opa/OpaClient.java Show resolved Hide resolved
...rvice/src/main/java/io/carbynestack/amphora/service/persistence/metadata/StorageService.java Outdated
public static final String SECRET_WITH_ID_EXISTS_EXCEPTION_MSG =
"A secret with the given id already exists.";
"A secret with the given id already exists.";
Copy link
Member Author

Choose a reason for hiding this comment

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

verify pre-commit formatting

...a-service/src/test/java/io/carbynestack/amphora/service/calculation/SecretShareUtilTest.java Outdated
Copy link
Member Author

Choose a reason for hiding this comment

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

undo

strieflin
strieflin requested changes Dec 16, 2024
View reviewed changes
Copy link
Member

@strieflin strieflin left a comment

Choose a reason for hiding this comment

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

Found only a few smaller things.

amphora-service/charts/amphora/values.yaml Outdated Show resolved Hide resolved
amphora-service/src/main/java/io/carbynestack/amphora/service/opa/OpaClient.java Outdated Show resolved Hide resolved
amphora-service/src/test/resources/application-test.properties Outdated Show resolved Hide resolved
amphora-service/src/test/java/io/carbynestack/amphora/service/opa/JwtReaderTest.java Outdated Show resolved Hide resolved
@sbckr
chore(service): address PR findings
f113daa 
Signed-off-by: Sebastian Becker <[email protected]>
@sbckr sbckr requested a review from strieflin December 17, 2024 08:02
strieflin
strieflin approved these changes Dec 17, 2024
View reviewed changes
Copy link
Member

@strieflin strieflin left a comment

Choose a reason for hiding this comment

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

LGTM

@sbckr sbckr merged commit 4aeac00 into master Dec 17, 2024
7 checks passed
@sbckr sbckr deleted the authorization branch December 17, 2024 08:42
This was referenced Dec 17, 2024
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@strieflin strieflin strieflin approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sbckr @strieflin