Skip to content

Restrict access to subnets for DB #31

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

Merged
merged 6 commits into from
Dec 18, 2018

Conversation

timja
Copy link
Contributor

@timja timja commented Dec 6, 2018

JIRA link

https://tools.hmcts.net/jira/browse/RDO-2675

Change description

This is an alternative approach to #30

Opening this up to review though

Does this PR introduce a breaking change? (check one with "x")

[x] Yes
[ ] No

db_rules = "${null_resource.subnet_mappings.*.triggers}"
}

# https://gist.github.com/brikis98/f3fe2ae06f996b40b55eebcb74ed9a9e
Copy link
Contributor

Choose a reason for hiding this comment

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

remove this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

don't mind, it's a link to where I got the weird terraform thingy


### Access to databases

Databases are restricted to access from specific subnets, these can be updated [here](https://github.com/hmcts/cnp-database-subnet-whitelisting)
Copy link
Contributor

Choose a reason for hiding this comment

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

I can't see where in the code it gets the json from this repo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Change description says this is pending 😉
It's been tested with the file directly in the repo,

Just swapping over to that atm

Copy link
Contributor

Choose a reason for hiding this comment

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

ah sorry I skipped straight to the code :)

find-subnets.py Outdated

# get first element or empty list if none
env_subnets = next(iter(env_subnets_list_of_lists), [])
app_subs = next(iter(app_subnets_list_of_lists), [])
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be cleaner to write:
env_subnets = env_subnets_list_of_lists[0] if len(env_subnets_list_of_lists) > 0 else []
but it's up to you really

@njrich28
Copy link
Contributor

njrich28 commented Dec 7, 2018

This is much more maintainable - clean separation of concerns mains that changes can be deployed cleanly across environments. Changes to environments and infrastructure no longer require deep understanding of the code.

@timja timja removed the do not merge label Dec 9, 2018
@timja timja mentioned this pull request Dec 11, 2018
@constantinpr constantinpr merged commit bc35b27 into master Dec 18, 2018
@timja timja deleted the feature/restrict-access-to-dbs branch December 2, 2019 09:19
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.

5 participants