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

Update documentation on GCS bundles #6264

Merged

Conversation

dennisg
Copy link
Contributor

@dennisg dennisg commented Oct 1, 2023

When an object in GCS contains special characters such as slashes (/) these need to be url-encoded in the configuration. If not, the bundle will not be found.

e.g. bundles/bundle.tar.gz should be entered as bundles%2fbundle.tar.gz

This PR adds notes to help the reader know about this and corrects some existing (IMHO incorrect) documentation.

Why the changes in this PR are needed?

I tried to add a bundle using GCS (with a bundle in a subfolder) and stumbled upon the fact myself. Took me some time
to figure it out. I hope this helps the next reader of this part of the documentation.

What are the changes in this PR?

Just some notes were added to the documentation regarding GCS bundles, some other parts were corrected.

Notes to assist PR review:

Personally, I think this is enough. If preferred, though, we could elaborate a little more on it and e.g. link to
the GCS documentation page.

Further comments:

relevant GCS docs: https://cloud.google.com/storage/docs/request-endpoints#encoding

When an object in GCS contains special characters such as slashes (/) these
need to be url-encoded in the configuration. If not, the bundle will not be found.

e.g. `bundles/bundle.tar.gz` should be entered as `bundles%2fbundle.tar.gz`

This PR adds a small note to help the reader know about this.

Signed-off-by: Dennis Geurts <dennisg@dennisg.nl>
@dennisg dennisg changed the title Update documentation on GCP bundles Update documentation on GCS bundles Oct 1, 2023
@netlify
Copy link

netlify bot commented Oct 1, 2023

Deploy Preview for openpolicyagent ready!

Name Link
🔨 Latest commit 6933802
🔍 Latest deploy log https://app.netlify.com/sites/openpolicyagent/deploys/65199a2b46fef00008dd7b30
😎 Deploy Preview https://deploy-preview-6264--openpolicyagent.netlify.app/docs/edge/management-bundles
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Signed-off-by: Dennis Geurts <dennisg@dennisg.nl>
@@ -941,6 +941,9 @@ bundles:
resource: 'bundle.tar.gz?alt=media'
```

If the resource (the object in the gcs bucket) contains slashes (/) or other special characters, these need to be url-encoded here, e.g.
`bundles/bundle.tar.gz?alt=media` should be entered as `bundles%2fbundle.tar.gz?alt=media`.
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, I added the link

@@ -631,13 +631,15 @@ services:
bundles:
authz:
service: gcs
resource: 'bundle.tar.gz?alt=media'
resource: 'bundles%2fhttp%2fexample%2fbundle.tar.gz?alt=media'
Copy link
Member

Choose a reason for hiding this comment

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

I don't think think the resource name should be changed just for the sake of using encoding. The previous example on line 500 is enough to do that, IMHO.

Copy link
Contributor Author

@dennisg dennisg Oct 1, 2023

Choose a reason for hiding this comment

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

Ok, I did that specifically because the Cloud Run example above actually uses the same resource as I mention here; there the encoding is not required. This, to me, drives home better the distinction between both types of GCP resources. But of course, we can change this back to the original.

Copy link
Member

Choose a reason for hiding this comment

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

No, that's fine. Thanks for helping out with the docs!

Signed-off-by: Dennis Geurts <dennisg@dennisg.nl>
@anderseknert anderseknert merged commit b9f2e89 into open-policy-agent:main Oct 1, 2023
23 of 24 checks 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.

None yet

2 participants