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: pictrs-safety #175

Merged
merged 4 commits into from
Oct 5, 2023
Merged

feat: pictrs-safety #175

merged 4 commits into from
Oct 5, 2023

Conversation

db0
Copy link
Contributor

@db0 db0 commented Sep 23, 2023

Adds inventory variable to enable picts-safety

pictrs gets an extra conditional for a new variable PICTRS__MEDIA__EXTERNAL_VALIDATION to connect the validation to pictrs-safety.

I used `{{ domain }} for it, as I don't know how otherwise to tell pictrs which IP to connect. Feel free to adjust to use some internal IP. My docker knowledge is very superficial.

Likewise, the pictrs-safety port is supposed to be exposed to the internet to allow workers to connect to it.

Currently I'm using the :main version as I'm having trouble pushing versioned docker images to ghcr.io. I'm trying to figure that out.

@db0 db0 mentioned this pull request Sep 23, 2023
@codyro codyro linked an issue Sep 23, 2023 that may be closed by this pull request
Copy link
Collaborator

@codyro codyro left a comment

Choose a reason for hiding this comment

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

I'll give it a more in-depth look later :).

Thanks for the PR!

@ticoombs ticoombs marked this pull request as ready for review September 28, 2023 04:37
@ticoombs ticoombs marked this pull request as draft September 28, 2023 04:37
@ticoombs
Copy link
Collaborator

From a code standpoint it looks fine now.

@db0 db0 marked this pull request as ready for review September 28, 2023 06:07
Copy link
Member

@dessalines dessalines left a comment

Choose a reason for hiding this comment

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

@codyro once it looks good to you, feel free to merge.

@dessalines dessalines requested a review from codyro September 28, 2023 12:36
@codyro codyro self-assigned this Oct 2, 2023
@codyro codyro added the enhancement New feature or request label Oct 2, 2023
@ticoombs ticoombs added this to the 1.3.0 milestone Oct 4, 2023
@codyro codyro merged commit b5c2e92 into LemmyNet:main Oct 5, 2023
@codyro
Copy link
Collaborator

codyro commented Oct 5, 2023

Everything looks good/tested on the normal targets.

At some point, we should consider showing an example of using SSL/TLS so the pictrs-safety<->worker data can be encrypted (image body is sent).

@db0 db0 deleted the pictrs-safety branch October 5, 2023 21:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add pictrs-safety?
5 participants