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

Added support for internal CIDR #2389

Merged
merged 2 commits into from
Oct 25, 2019
Merged

Added support for internal CIDR #2389

merged 2 commits into from
Oct 25, 2019

Conversation

jmpsf
Copy link
Contributor

@jmpsf jmpsf commented Oct 21, 2019

Right now the ec2 instances created by molecule allow an ingress rule to 0.0.0.0/0 on the ssh port.

As we have a restrictive security policy, we only allow ssh traffic from within the subnet, therefore we need to be able to specify the CIDR that is allowed to ssh into the molecule ec2 instance.

As there can be multiple platforms I've decided to set up the configuration in the molecule.yml under the driver section. Although I don't know if it's the best place.

driver:
  name: ec2
  internal_cidr: 192.168.100.0/24

Changes are backward compatible and shouldn't affect current users.

PR Type

  • Feature Pull Request

Signed-off-by: Josetxu <jmp@icij.org>
Copy link
Contributor

@decentral1se decentral1se left a comment

Choose a reason for hiding this comment

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

I can't test this and we have no functional tests right now for this driver (I think!).

If you can assure this is working for you TM, then we can see about merging it.

Quite "best effort" at the moment for this driver testing.

@jmpsf
Copy link
Contributor Author

jmpsf commented Oct 22, 2019

Hi @decentral1se, I've checked both cases, either specifying the internal_cidr and not specifying it and it worked as expected

Copy link
Member

@ssbarnea ssbarnea left a comment

Choose a reason for hiding this comment

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

Is there any reason why attribute name used by module is different that attribute name passed down?

I find as future source of confusions, so unless there is a good reason to name it internal_cidr instead of cidr_ip please fix that.

Signed-off-by: Josetxu <jmp@icij.org>
@jmpsf jmpsf requested a review from ssbarnea October 22, 2019 15:11
@jmpsf
Copy link
Contributor Author

jmpsf commented Oct 23, 2019

@ssbarnea I've just changed the names of the variables, I don't see how it has broken the tests, could we rerun them?

@decentral1se
Copy link
Contributor

Restarted the failed build. It was unrelated.

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

3 participants