Ease modification of base image

[spawnia]

Resolves #1477
Resolves #882

Provides a simple and canonical way to make modifications of the Sentry base image, such as installing plugins or their dependencies.

In contrast to previous solutions, this method:

• applies to all services using the Sentry base image (see Install plugins in web and sentry-cleanup #1477)
• runs exactly once during install.sh for all services
• does not require modification of setup files within the repository

Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

[aminvakil]

I would say let's keep this PR to moving from using sentry image to building sentry image and do stuff after.

And open another PRs for other modifications.

[BYK]

Looks pretty great so far, thanks for putting this effort in. I'll leave the final approval to @chadwhitacre (he needs to approve to land anyway).

That said please address my comments regarding test.sh file as otherwise you'll break builds in all other Sentry repos :) -- @chadwhitacre we should probably add a simple GHA check for that here instead of using me as a human e2e/lint check 😅

[BYK]

LGTM, great work thanks a lot 👏🏻

[chadwhitacre]

Thanks for the contribution @spawnia and for reviewing @BYK @aminvakil!

Kicking off CI ...

/gcbrun

[chadwhitacre]

The main purpose of factoring this out is to make it easier to run locally, yes? (cf. CONTRIBUTING.md). In that case, how does ./integration-test.sh relate to ./test.sh? Looks like there has been some back and forth on this with 004d41b. :)

[spawnia]

The main purpose of factoring this out is to make it easier to run locally, yes?

Exactly.

In that case, how does ./integration-test.sh relate to ./test.sh?

I am not sure how to answer this. I think their purpose follows from their contents and usage in the project. Would you like to rename them or comment them?

[chadwhitacre]

Almost certainly a way to DRY this up.

[spawnia]

I am sure there is, I am not convinced it is worth the effort though. I do follow DRY, but also WET - write everything twice. Given this is a negation, we will never need to maintain more than two versions of this.

[chadwhitacre]

Thanks for the PR! Some initial questions, not sure we're there yet on how test code is organized between the different scripts ... but I think we can get there. :)

[chadwhitacre]

/gcbrun

[chadwhitacre]

Thanks @spawnia! Merging as-is, I think there are some slight improvements possible but don't want to hold this up further. :)

[spawnia]

Thanks to everyone involved. Looking forward to simplifying our setup with this.

[PMExtra]

Great job!

But why do we have to migrate from requirements.txt to enhance-image.sh ?

I think they can coexist. In some cases, use requirements.txt is simpler than shell script.

I think we can drop the requirements.txt is deprecated message, then everything will be awesome.

[spawnia]

I think the simplicity of having a single shell script is preferrable.

requirements.txt pretty much only saves the addition of pip install, but at the added cost of:

• having an extra file
• having to document two ways of achieving the same thing
• missing clarity as to the order of execution - one more thing to think about or document

[PMExtra]

@spawnia OK, you're right.
I used to think that shell is not a "pretty" format to specify multiple packages and their version constraints.
But now I found a solution like that:

requirements=(
sentry-auth-ldap==21.9.*
sentry-tablestore==git+https://github.com/PMExtra/sentry-tablestore.git@master
)

pip install ${requirements[@]}

[chadwhitacre]

Nice hack @PMExtra. :)