Skip to content

Conversation

@Spice-King
Copy link
Contributor

Mount a certificate folder to local ca storage in containers,
and add update command to cron image's entrypoint.

Result of poking and prodding from getsentry/sentry#26851

@BYK
Copy link
Member

BYK commented Jul 1, 2021

Thanks so much for doing this and also submitting a patch for the docs!

@BYK BYK self-assigned this Jul 1, 2021
@BYK
Copy link
Member

BYK commented Jul 1, 2021

Well also, seeing getsentry/develop#369 reminded me that we now have a CHANGELOG here so it would be great to mention this here with a link to the new docs: https://github.com/getsentry/onpremise/blob/master/CHANGELOG.md

@Spice-King
Copy link
Contributor Author

Well also, seeing getsentry/develop#369 reminded me that we now have a CHANGELOG here so it would be great to mention this here with a link to the new docs: https://github.com/getsentry/onpremise/blob/master/CHANGELOG.md

Did that, rebased and updated all the repos.

Copy link
Member

@BYK BYK left a comment

Choose a reason for hiding this comment

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

Looks pretty good, holding until we figure out what to do with sentry/onpremise repo split and tests.

@BYK BYK requested a review from chadwhitacre July 8, 2021 11:45
@Spice-King
Copy link
Contributor Author

Been busy, lost the notifications in the noise. Sorry about that. Going to quickly rebase and double check stuff.

@chadwhitacre chadwhitacre requested a review from BYK July 13, 2021 11:33
@chadwhitacre
Copy link
Member

I rebased this now that 21.7.0 is out.

@chadwhitacre
Copy link
Member

Seeing a test failure:

python3: can't open file '/etc/sentry/cert-test.py': [Errno 2] No such file or directory

Seeing a weirder test failure locally, not sure what's up. 🤔

./test/core.sh: line 90: DSN_PIECES[0]: unbound variable

Mount a certificate folder to local ca storage in containers,
and add update command to cron image's entrypoint.
@chadwhitacre
Copy link
Member

Seeing a weirder test failure locally, not sure what's up.

Sorted that out in #1031, rebased again.

@chadwhitacre
Copy link
Member

Seeing a test failure:

Did a local refactor, and now back to seeing this failure locally.

@chadwhitacre
Copy link
Member

Alright, I think I've almost wrapped my head around this via the refactor in 600becb. Not quite passing yet but I think I'm close. Gonna pause there for the weekend, will revisit on Monday and see where things stand. The approach seems straightforward, I appreciate the effort to test, would love to get that across the finish line.

@chadwhitacre
Copy link
Member

chadwhitacre commented Jul 19, 2021

Did a little more refactoring, I'm now focused on achieving the same failure in both local and CI. From there I/we can figure out how to fix the failure. :)

- Match docker-compose file versions - Not getting an error locally with
  docker-compose 1.29, but I am seeing one in CI where we are pinned to
1.24.

- Drop scale usage - `scale` is a v2 construct. Merged v2/v3 support was
  added in docker-compose 1.27, but we're still pinned to 1.24 in CI.
@chadwhitacre
Copy link
Member

@Spice-King Okay, I've got the same result in CI and local. I'll keep poking at is as I'm able, but if you get to it before I do ... could you try out the latest locally for you and see if you get the same result? My hunch is that I boogered something up in all of my refactoring, hopefully it's a quick fix. :)

Custom CA cert failed to work.
  Traceback (most recent call last):
    File "/usr/local/lib/python3.6/site-packages/urllib3/connectionpool.py", line 600, in urlopen
      chunked=chunked)
    File "/usr/local/lib/python3.6/site-packages/urllib3/connectionpool.py", line 343, in _make_request
      self._validate_conn(conn)
    File "/usr/local/lib/python3.6/site-packages/urllib3/connectionpool.py", line 839, in _validate_conn
      conn.connect()
    File "/usr/local/lib/python3.6/site-packages/urllib3/connection.py", line 344, in connect
      ssl_context=context)
    File "/usr/local/lib/python3.6/site-packages/urllib3/util/ssl_.py", line 347, in ssl_wrap_socket
      return context.wrap_socket(sock, server_hostname=server_hostname)
    File "/usr/local/lib/python3.6/ssl.py", line 407, in wrap_socket
      _context=self, _session=session)
    File "/usr/local/lib/python3.6/ssl.py", line 817, in __init__
      self.do_handshake()
    File "/usr/local/lib/python3.6/ssl.py", line 1077, in do_handshake
      self._sslobj.do_handshake()
    File "/usr/local/lib/python3.6/ssl.py", line 689, in do_handshake
      self._sslobj.do_handshake()
  ssl.SSLError: [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed (_ssl.c:852)

@chadwhitacre
Copy link
Member

P.S. The repro steps are:

cd onpremise
git pull
./_integration-test/run.sh

@Spice-King
Copy link
Contributor Author

@chadwhitacre I have no clue why it's opted to ignore the environment variable when it accepted it before. Spent my while evening on it. I'll prod my work's instance I altered in a crude way using a pile of overrides. Last I checked, it works there. If all less failed, I do have an idea that should work, but is more ham fisted than what I'd have liked. The "It can't be tight if it's a liquid" kind of option on the Tool Expectations chart.

@chadwhitacre
Copy link
Member

Huzzah! I'm able to manually set up a successful request locally. Will see about test/CI ...

root@256dce7eda3d:/usr/local/share/ca-certificates# python /etc/sentry/test-custom-ca-roots.py 
ok
root@256dce7eda3d:/usr/local/share/ca-certificates#

@chadwhitacre
Copy link
Member

Alright, we're green! @BYK You had some feedback on this approach?

@chadwhitacre
Copy link
Member

Hrm, 5708586ed0af10c2a9d22fd3de93f3e755aebcd7 passes locally with this diff. 🤔

diff --git a/.env b/.env
index d49dae9..8ba24c3 100644
--- a/.env
+++ b/.env
@@ -3,7 +3,7 @@ SENTRY_EVENT_RETENTION_DAYS=90
 # You can either use a port number or an IP:PORT combo for SENTRY_BIND
 # See https://docs.docker.com/compose/compose-file/#ports for more
 SENTRY_BIND=9000
-SENTRY_IMAGE=getsentry/sentry:nightly
+SENTRY_IMAGE=us.gcr.io/sentryio/sentry:16f56945390034929c159ac0dab81584602eb65c
 SNUBA_IMAGE=getsentry/snuba:nightly
 RELAY_IMAGE=getsentry/relay:nightly
 SYMBOLICATOR_IMAGE=getsentry/symbolicator:nightly

@BYK BYK closed this Jul 29, 2021
@BYK BYK reopened this Jul 29, 2021
@BYK BYK requested a review from chadwhitacre July 30, 2021 15:29
@BYK
Copy link
Member

BYK commented Jul 30, 2021

Okay, I think this is good for a final review @chadwhitacre and @Spice-King

Copy link
Member

@chadwhitacre chadwhitacre left a comment

Choose a reason for hiding this comment

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

Ship it! 🚢

@BYK BYK merged commit 17b675c into getsentry:master Jul 30, 2021
BYK pushed a commit to getsentry/develop that referenced this pull request Jul 30, 2021
BYK added a commit to getsentry/snuba that referenced this pull request Jul 30, 2021
BYK added a commit to getsentry/snuba that referenced this pull request Jul 30, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Aug 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants