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.

Sign up for GitHub

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

Jump to bottom

Allow multiple hostname #2733

Merged
merged 4 commits into from
Jun 1, 2022
Merged

Allow multiple hostname #2733

merged 4 commits into from
Jun 1, 2022

Conversation

pratixha
Copy link
Contributor

@pratixha pratixha commented May 13, 2022 •
edited
Loading

Signed-off-by: pratixha pratiksha.prajapati@msystechnologies.com

Description

#2698
Add support for adding multiple hostnames.

We use more than one DNS name for our supermarket, primarily for staging purposes.

e.g. supermarket.myorg.net & supermarket.prd.myteam.myorg.net

Issues Resolved

[List any existing issues this PR resolves, or any Discourse or
StackOverflow discussions that are relevant]

Check List

@pratixha pratixha requested review from a team as code owners May 13, 2022 11:36
@github-actions github-actions bot added the Documentation Gets work onto the docs board label May 13, 2022
@pratixha pratixha force-pushed the pratiksha/allow-multiple-hostnames branch 4 times, most recently from 3f5f49b to e9c5478 Compare May 13, 2022 12:25
@chef chef deleted a comment from netlify bot May 13, 2022
@RajeshPaul38
Copy link
Contributor

This change should not be about setting multiple hostnames against supermarket but it's about allowing multiple hosts in the host header. So we should only change allowed_host attribute instead of hostname @pratixha

@pratixha pratixha force-pushed the pratiksha/allow-multiple-hostnames branch from e9c5478 to 611b16b Compare May 16, 2022 05:02
RajeshPaul38
RajeshPaul38 requested changes May 16, 2022
View reviewed changes
Copy link
Contributor

@RajeshPaul38 RajeshPaul38 left a comment

Choose a reason for hiding this comment

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

@pratixha pls check these. Also make sure throughout the codebase allowed_host is changed to allowed_hosts

docs-chef-io/content/supermarket/config_rb_supermarket.md Outdated Show resolved Hide resolved
docs-chef-io/content/supermarket/config_rb_supermarket.md Outdated Show resolved Hide resolved
docs-chef-io/content/supermarket/config_rb_supermarket.md Outdated Show resolved Hide resolved
src/supermarket/config/environments/production.rb Outdated Show resolved Hide resolved
src/supermarket/config/environments/production.rb Outdated Show resolved Hide resolved
omnibus/cookbooks/omnibus-supermarket/attributes/default.rb Outdated Show resolved Hide resolved
@pratixha pratixha force-pushed the pratiksha/allow-multiple-hostnames branch from 611b16b to 90cacd4 Compare May 16, 2022 05:47
@pratixha
Copy link
Contributor Author

@pratixha pls check these. Also make sure throughout the codebase allowed_host is changed to allowed_hosts

Thanks @RajeshPaul38 for correction, please check and review the same.

@pratixha
Support multiple hosts with allowed_hosts attribute
5344c21 
Signed-off-by: pratixha <pratiksha.prajapati@msystechnologies.com>
@pratixha pratixha force-pushed the pratiksha/allow-multiple-hostnames branch from 90cacd4 to 5344c21 Compare May 16, 2022 05:52
@pratixha
Replaced allowed_host with allowed_hosts
1dd6881 
Signed-off-by: pratixha <pratiksha.prajapati@msystechnologies.com>
msys-sgarg
msys-sgarg previously approved these changes May 17, 2022
View reviewed changes
Copy link
Contributor

@msys-sgarg msys-sgarg left a comment

Choose a reason for hiding this comment

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

Changes look good to me

@msys-sgarg msys-sgarg self-requested a review May 17, 2022 09:15
@msys-sgarg msys-sgarg dismissed their stale review May 17, 2022 09:16

This PR needs an adhoc build and an end to end testing on a new ec2 instance before being approved

@pratixha
Copy link
Contributor Author

Adhoc buld:-
https://buildkite.com/chef/chef-supermarket-main-omnibus-adhoc/builds/505

@RajeshPaul38 RajeshPaul38 mentioned this pull request May 19, 2022
Closed
5 tasks
RajeshPaul38
RajeshPaul38 approved these changes May 19, 2022
View reviewed changes
Copy link
Contributor

@RajeshPaul38 RajeshPaul38 left a comment

Choose a reason for hiding this comment

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

LGTM

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@RajeshPaul38 RajeshPaul38 mentioned this pull request May 23, 2022
Open
@RajeshPaul38
Copy link
Contributor

@saghoshprogress can you pls check the wordings of the doc and developer comments and approve if it looks good.

@RajeshPaul38 RajeshPaul38 requested review from saghoshprogress and removed request for msys-sgarg June 1, 2022 05:19
@saghoshprogress saghoshprogress merged commit f99c27f into main Jun 1, 2022
@saghoshprogress saghoshprogress deleted the pratiksha/allow-multiple-hostnames branch June 1, 2022 05:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ayush-billore ayush-billore ayush-billore approved these changes

@RajeshPaul38 RajeshPaul38 RajeshPaul38 approved these changes

@saghoshprogress saghoshprogress saghoshprogress approved these changes

@msys-sgarg msys-sgarg msys-sgarg left review comments

Assignees
No one assigned
Labels
Documentation Gets work onto the docs board
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@pratixha @RajeshPaul38 @msys-sgarg @saghoshprogress @ayush-billore