Skip to content
This repository has been archived by the owner on Jan 30, 2024. It is now read-only.

Add whitehall govuk_basic_auth_credentials to link checker api for integration & staging #7092

Merged
merged 1 commit into from Jan 16, 2018

Conversation

SebAshton
Copy link
Contributor

This PR adds a new env var to set the basic auth credentials for the link checker API to pass the basic auth challenge on integration.

@SebAshton SebAshton force-pushed the add-govuk-basic-auth-credentials branch from d98546e to bc81117 Compare January 16, 2018 10:34
@SebAshton SebAshton changed the title Add whitehall basic_auth_credentials for integration Add whitehall govuk_basic_auth_credentials to link checker api for integration & staging Jan 16, 2018
@SebAshton SebAshton force-pushed the add-govuk-basic-auth-credentials branch from bc81117 to a2376b1 Compare January 16, 2018 10:41
Copy link
Contributor

@thomasleese thomasleese left a comment

Choose a reason for hiding this comment

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

I think you might have missed the update to integration in this PR.

@@ -25,6 +25,7 @@ govuk::apps::email_alert_api::govuk_notify_base_url: 'https://api.staging-notify
govuk::apps::email_alert_api::govuk_notify_template_id: '76d21ce7-54c3-4fb7-8830-ba3b79287985'
govuk::apps::hmrc_manuals_api::publish_topics: false
govuk::apps::kibana::logit_environment: d414187a-2796-4ea7-9b9a-d40c341646d6
govuk::apps::link_checker_api::govuk_basic_auth_credentials: "%{hiera('http_username')}:%{hiera('http_password')}"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this will need the Basic at the beginning of the string.

Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason we haven't split these up as per previous use cases:

govuk_jenkins::jobs::search_benchmark::auth_username: "%{hiera('http_username')}"

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually... since we called the variable govuk_basic_auth_credentials, I think we should update https://github.com/alphagov/link-checker-api/pull/120/files#diff-4e80c9e759124f64feef221083d041dbR294 to put in the Basic for us.

Copy link
Contributor

Choose a reason for hiding this comment

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

@dwhenry No real reason except it fits what Whitehall used to do and it works better with how the Link Checker API is configured.

@SebAshton SebAshton force-pushed the add-govuk-basic-auth-credentials branch 2 times, most recently from 09d5ea9 to 4c4379d Compare January 16, 2018 10:56
@@ -25,6 +25,7 @@ govuk::apps::email_alert_api::govuk_notify_base_url: 'https://api.staging-notify
govuk::apps::email_alert_api::govuk_notify_template_id: '76d21ce7-54c3-4fb7-8830-ba3b79287985'
govuk::apps::hmrc_manuals_api::publish_topics: false
govuk::apps::kibana::logit_environment: d414187a-2796-4ea7-9b9a-d40c341646d6
govuk::apps::link_checker_api::govuk_basic_auth_credentials: "Basic %{hiera('http_username')}:%{hiera('http_password')}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry I've changed my mind... I think we shouldn't include the Basic in the string here as the variable has basic in the name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🙃

@SebAshton SebAshton force-pushed the add-govuk-basic-auth-credentials branch from 4c4379d to accd0c8 Compare January 16, 2018 11:16
@SebAshton SebAshton merged commit 7549c3d into master Jan 16, 2018
@SebAshton SebAshton deleted the add-govuk-basic-auth-credentials branch January 16, 2018 11:34
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.

None yet

3 participants