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 SAML & LDAP integrations for on-prem #16239

Merged

Conversation

rafatower
Copy link
Contributor

@shortcut-integration
Copy link

This pull request has been linked to Clubhouse Story #144728: Fix SAML Integration for onprem.

@rafatower rafatower changed the title Fix SAML Integration for onprem Fix SAML & LDAP integrations for on-prem Mar 31, 2021
@rafatower rafatower force-pushed the feature/ch144728/fix-saml-ldap-integration-for-onprem branch from bef0d05 to 4166a44 Compare March 31, 2021 07:41
@rafatower
Copy link
Contributor Author

rafatower commented Mar 31, 2021

@amiedes the test build timed out after 40 min

FATAL:  the database system is shutting down

EDIT: link to the logs https://console.cloud.google.com/cloud-build/builds;region=global/100743d8-e0c5-404a-ace8-de02ba5601b4;step=12?project=cartodb-on-gcp-ci-testing

@rafatower rafatower requested a review from amiedes March 31, 2021 09:20
Copy link
Contributor

@amiedes amiedes left a comment

Choose a reason for hiding this comment

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

👍

@rafatower rafatower force-pushed the feature/ch144728/fix-saml-ldap-integration-for-onprem branch from 9d56e95 to 0c15817 Compare March 31, 2021 14:26
@rafatower
Copy link
Contributor Author

Closer to fix the tests problem:

$ cat match-pairs.rb
INPUT_FILE='log-1ec7bd34-a1e3-4530-a35a-447aa6744b4f.txt'
START_RE = /RTORRE Starting example (.*)/
FINISH_RE = /RTORRE Finishing example (.*)/ 

opened_examples = {}
File.readlines(INPUT_FILE).each do |line|
  m = line.match(START_RE)
  if m
    example = m[1]
    opened_examples[example] = true
  end

  m = line.match(FINISH_RE)
  if m
    example = m[1]
    opened_examples.delete(example)
  end
end

puts opened_examples.keys

$ ruby match-pairs.rb
Carto::UserCreation validation token neither creates a new User nor sends the mail and marks creation as failure if Central fails

@rafatower
Copy link
Contributor Author

NOTE: The ruby timeouts do not work 100% of the time, definitely not in this case, I think because of the threading model, they cannot interrupt the ruby process when it is in a "native" section (most likely sequel pg client waiting for db)

@rafatower
Copy link
Contributor Author

@amiedes I'm pretty sure it's that evil until provoking the stuck situation:

user_creation.next_creation_step until user_creation.finished?

I mean, I think I know now how to fix it, but a CI is not very robust if we can get it stuck like that.

@rafatower rafatower force-pushed the feature/ch144728/fix-saml-ldap-integration-for-onprem branch 2 times, most recently from c449ec6 to c694700 Compare March 31, 2021 15:44
This happens when a new SAML or LDAP user signs up and it is not yet
created in cartodb. It results in a sync call to Central HTTP API.

This avoids such call when the Central api is not enabled (notably in
on-premise installations).

Also this fixes tests that otherwise wait forever on an `until`.
@rafatower rafatower force-pushed the feature/ch144728/fix-saml-ldap-integration-for-onprem branch from c694700 to 0054534 Compare April 5, 2021 07:04
@rafatower
Copy link
Contributor Author

I decided to remove the timeouts code in tests ( 95a1506 and bf573f5 ) because it does not really work and it can be misleading.

@rafatower rafatower merged commit e5a9bd6 into master Apr 5, 2021
@rafatower rafatower deleted the feature/ch144728/fix-saml-ldap-integration-for-onprem branch April 5, 2021 07:28
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