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

fix(egress/record): rename capability #1572

Merged
merged 7 commits into from
Nov 5, 2024
Merged

Conversation

fforbeck
Copy link
Member

@fforbeck fforbeck commented Oct 31, 2024

Update to usage/record Capability

Overview

This PR restructures the usage/record capability, moving it under the Space namespace instead of Usage. As part of this change, the usage/record definition has been renamed to space/content/serve/egress/record, and a new top-level capability, space/content/serve/*, has been introduced.

Key Changes

  • Namespace Update: The usage/record capability now resides under the Space namespace.
  • New Naming Convention:
    • space/content/serve/egress/record: This capability records egress for all served data.
    • space/content/serve/*: New top-level capability, representing general serve actions within the Space.contentServe namespace.

@fforbeck fforbeck requested a review from travis October 31, 2024 18:46
@fforbeck fforbeck marked this pull request as ready for review November 1, 2024 18:19
Copy link
Member

@alanshaw alanshaw left a comment

Choose a reason for hiding this comment

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

As I mentioned before, I don't think this is the way we should do this. There's no delegatation chain back to the user. All we're doing here is using UCAN to verify we can call microservices within our own stack, for which we could simply use a plain HTTP API and a bearer token.

From discord: It's important that the gateway is authorized to record egress. In a future where there are multiple decentralized gateways, there should be no other gateway able to record egress usage for a given space unless authorized to do so. There should be a delegation chain from the user to the gateway that authorizes it to track egress usage so that the user can be billed accordingly.

I'm loathed to approve this - we're going to have to undo it in the future.

packages/upload-api/test/helpers/utils.js Outdated Show resolved Hide resolved
@fforbeck fforbeck changed the title fix(usage/record): add provider as the resource fix(usage/record): rename capability Nov 4, 2024
@fforbeck fforbeck changed the title fix(usage/record): rename capability fix(egress/record): rename capability Nov 5, 2024
@fforbeck
Copy link
Member Author

fforbeck commented Nov 5, 2024

As I mentioned before, I don't think this is the way we should do this. There's no delegatation chain back to the user. All we're doing here is using UCAN to verify we can call microservices within our own stack, for which we could simply use a plain HTTP API and a bearer token.

From discord: It's important that the gateway is authorized to record egress. In a future where there are multiple decentralized gateways, there should be no other gateway able to record egress usage for a given space unless authorized to do so. There should be a delegation chain from the user to the gateway that authorizes it to track egress usage so that the user can be billed accordingly.

I'm loathed to approve this - we're going to have to undo it in the future.

Thanks for flagging this @alanshaw ! I've updated the code and renamed the capability to make it easy for us in the future. This is ready for review again.

Copy link
Member

@alanshaw alanshaw left a comment

Choose a reason for hiding this comment

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

LGTM

packages/capabilities/src/space.js Outdated Show resolved Hide resolved
packages/upload-api/test/helpers/utils.js Outdated Show resolved Hide resolved
@fforbeck fforbeck merged commit d28691c into main Nov 5, 2024
6 checks passed
@fforbeck fforbeck deleted the fix/usage-record-definition branch November 5, 2024 19:16
fforbeck pushed a commit that referenced this pull request Nov 5, 2024
🤖 I have created a release *beep* *boop*
---


##
[18.1.1](upload-api-v18.1.0...upload-api-v18.1.1)
(2024-11-05)


### Fixes

* **egress/record:** rename capability
([#1572](#1572))
([d28691c](d28691c))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
fforbeck pushed a commit that referenced this pull request Nov 5, 2024
🤖 I have created a release *beep* *boop*
---


##
[16.4.1](w3up-client-v16.4.0...w3up-client-v16.4.1)
(2024-11-05)


### Fixes

* **egress/record:** rename capability
([#1572](#1572))
([d28691c](d28691c))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
fforbeck added a commit that referenced this pull request Nov 5, 2024
🤖 I have created a release *beep* *boop*
---


##
[17.4.1](capabilities-v17.4.0...capabilities-v17.4.1)
(2024-11-05)


### Fixes

* **egress/record:** rename capability
([#1572](#1572))
([d28691c](d28691c))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Felipe Forbeck <fforbeck@gmail.com>
fforbeck pushed a commit that referenced this pull request Nov 5, 2024
🤖 I have created a release *beep* *boop*
---


##
[16.4.1](w3up-client-v16.4.0...w3up-client-v16.4.1)
(2024-11-05)


### Fixes

* **egress/record:** rename capability
([#1572](#1572))
([d28691c](d28691c))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

3 participants