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

CDPCAM-71 Add Private Subnets validation with 'Auto-assign public IPs' disabled #22

Merged
merged 8 commits into from
Aug 24, 2021
Merged

CDPCAM-71 Add Private Subnets validation with 'Auto-assign public IPs' disabled #22

merged 8 commits into from
Aug 24, 2021

Conversation

anuragpatro
Copy link
Contributor

Signed-off-by: Anurag Patro [email protected]

Private Subnets required 'Auto-assign public IPs' option to be disabled since we are following a 'fully private' configuration for network architecture. Adding validation and tests for the same.

tests/validation/infra/test_validate_aws_subnets.py::test_aws_public_subnets_validation_failure PASSED [ 7%]
tests/validation/infra/test_validate_aws_subnets.py::test_aws_public_subnets_suffix_validation_success PASSED [ 10%]
tests/validation/infra/test_validate_aws_subnets.py::test_aws_public_subnets_suffix_validation_failure PASSED [ 14%]
tests/validation/infra/test_validate_aws_subnets.py::test_aws_public_subnets_availablity_zone_validation_success PASSED [ 17%]
tests/validation/infra/test_validate_aws_subnets.py::test_aws_public_subnets_availablity_zone_validation_failure PASSED [ 21%]
tests/validation/infra/test_validate_aws_subnets.py::test_aws_public_subnets_route_validation_success PASSED [ 25%]
tests/validation/infra/test_validate_aws_subnets.py::test_aws_public_subnets_route_validation_failure PASSED [ 28%]
tests/validation/infra/test_validate_aws_subnets.py::test_aws_public_subnets_range_validation_success PASSED [ 32%]
tests/validation/infra/test_validate_aws_subnets.py::test_aws_public_subnets_range_validation_failure PASSED [ 35%]
tests/validation/infra/test_validate_aws_subnets.py::test_aws_public_subnets_tags_validation_success PASSED [ 39%]
tests/validation/infra/test_validate_aws_subnets.py::test_aws_public_subnets_tags_validation_failure PASSED [ 42%]
tests/validation/infra/test_validate_aws_subnets.py::test_aws_private_subnets_validation_success PASSED [ 46%]
tests/validation/infra/test_validate_aws_subnets.py::test_aws_private_subnets_validation_failure PASSED [ 50%]
tests/validation/infra/test_validate_aws_subnets.py::test_aws_private_subnets_suffix_validation_success PASSED [ 53%]
tests/validation/infra/test_validate_aws_subnets.py::test_aws_private_subnets_suffix_validation_failure PASSED [ 57%]
tests/validation/infra/test_validate_aws_subnets.py::test_aws_private_subnets_availablity_zone_validation_success PASSED [ 60%]
tests/validation/infra/test_validate_aws_subnets.py::test_aws_private_subnets_availablity_zone_validation_failure PASSED [ 64%]
tests/validation/infra/test_validate_aws_subnets.py::test_aws_private_subnets_route_validation_success PASSED [ 67%]
tests/validation/infra/test_validate_aws_subnets.py::test_aws_private_subnets_route_validation_failure PASSED [ 71%]
tests/validation/infra/test_validate_aws_subnets.py::test_aws_private_subnets_range_validation_success PASSED [ 75%]
tests/validation/infra/test_validate_aws_subnets.py::test_aws_private_subnets_range_validation_failure PASSED [ 78%]
tests/validation/infra/test_validate_aws_subnets.py::test_aws_private_subnets_tags_validation_success PASSED [ 82%]
tests/validation/infra/test_validate_aws_subnets.py::test_aws_private_subnets_tags_validation_failure PASSED [ 85%]
tests/validation/infra/test_validate_aws_subnets.py::test_aws_private_subnets_auto_assign_ip_validation_success PASSED [ 89%]
tests/validation/infra/test_validate_aws_subnets.py::test_aws_private_subnets_auto_assign_ip_validation_failure PASSED [ 92%]
tests/validation/infra/test_validate_aws_subnets.py::test_aws_vpc_validation_success PASSED [ 96%]
tests/validation/infra/test_validate_aws_subnets.py::test_aws_vpc_validation_failure PASSED [100%]

=====================28 passed in 0.60s ===============================

@anuragpatro anuragpatro marked this pull request as draft August 20, 2021 11:16
@anuragpatro anuragpatro marked this pull request as ready for review August 20, 2021 11:17
@anuragpatro
Copy link
Contributor Author

@mahendra-rk @aakanser @wonderslug
Please review if the validation seems okay, thanks

@wonderslug
Copy link
Collaborator

I think the test is probably fine. But we are introducing warnings as well as failures and I think this is a good candidate to be a warning since I don't think it would actually effect of the provisioning of an environment fails or not.

Maybe we can just hold onto this till we get warning in in the next week or so and rebase and update it then?

@wonderslug wonderslug added the enhancement New feature or request label Aug 20, 2021
@wonderslug
Copy link
Collaborator

@anuragpatro Ok Warn and the new issues system is now in place in dev. If you can please rebase against dev and see the other validations for how the issues are being presented now with the fail and the warn functions. In order to get your Issue which you add to the issue_templates.yml file, you need to run the scripts/update_issue_templates.py. This will add a const for the issue in the issues.py to use for references.

wonderslug and others added 6 commits August 24, 2021 09:14
@wonderslug @anuragpatro
CDPCAM-48, CDPCAM-43: Custom output and links to docs. (#23)
1b440de 
* CDPCAM-48, CDPCAM-43: Custom output and links to docs.

This commits provides the capability to do different output types
(tex and json for now) and provides the different issues encountered
with the ability to have links and different formatting.

It does this through centralizing the Issue messages into
issue_templates.yml files which get loaded at runtime to present the issues
to the renderers.  The renderers pick the formatting mechanism defined by the
render_type in the issue_templates.yml.

The issue ids can be generated into constants using the
scripts/update_issue_templates.py script.  This will also be run during
the pre-commit.

Signed-off-by: Brian Towles <[email protected]>
@wonderslug @anuragpatro
Fix for package install in docker image
e926947
@wonderslug @anuragpatro
CDPCAM-43: Added more links to the Issue Templates (#24)
a9852b8 
This commits adds more links to the issue_templates.yml files.

Signed-off-by: Brian Towles <[email protected]>
@wonderslug @anuragpatro
CDPCAM-72: Added warnings support (#25)
cdff940 
This commit adds warnings support in the validations
output as well for the testing.  It also changes some
experience specific validations to warnings.

Signed-off-by: Brian Towles <[email protected]>
@anuragpatro
adding subnet auto-assign ip warning with addition to issues
22997d8 
Signed-off-by: Anurag Patro <[email protected]>
@anuragpatro
Merge branch 'dev' into subnet_ip
66b7774
@anuragpatro
Copy link
Contributor Author

Made the necessary changes, please review.
One of your commit seems to be missing the sign-off, that's raising an error, if that can be forced through (if maintainers have that ability), should be fine else please tell me how to bypass that.

@codecov-commenter
Copy link

codecov-commenter commented Aug 24, 2021 •
edited
Loading

Codecov Report

Merging #22 (c74d24d) into dev (739d9b9) will increase coverage by 0.09%.
The diff coverage is 85.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev      #22      +/-   ##
==========================================
+ Coverage   70.27%   70.37%   +0.09%     
==========================================
  Files          27       27              
  Lines        1228     1249      +21     
==========================================
+ Hits          863      879      +16     
- Misses        365      370       +5
Flag Coverage Δ
unittests 70.37% <85.71%> (+0.09%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
cdpctl/validation/infra/validate_aws_subnets.py 86.69% <84.61%> (-0.13%) ⬇️
cdpctl/validation/infra/issues.py 100.00% <100.00%> (ø)
cdpctl/command/validate.py 0.00% <0.00%> (ø)
cdpctl/validation/__init__.py 91.91% <0.00%> (+0.93%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 739d9b9...c74d24d. Read the comment docs.

@wonderslug
Copy link
Collaborator

Looks like a couple of minor lint and isort issues. If you can please fix we can get these in.

You can check these locally before you commit by doing a pre-commit run --all inside the dev container terminal.

Other then that looking good. Not sure why there is the DCO issue.

mahendra-rk
mahendra-rk approved these changes Aug 24, 2021
View reviewed changes
Copy link
Collaborator

@mahendra-rk mahendra-rk left a comment

Choose a reason for hiding this comment

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

LGTM!

@mahendra-rk
Copy link
Collaborator

Please run lint and add necessary fixes

@anuragpatro
lint fixes
c74d24d 
Signed-off-by: Anurag Patro <[email protected]>
@wonderslug wonderslug merged commit e7a0089 into cloudera-labs:dev Aug 24, 2021
@wonderslug
Copy link
Collaborator

Thanks for the validation!

@anuragpatro anuragpatro deleted the subnet_ip branch August 24, 2021 20:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mahendra-rk mahendra-rk Awaiting requested review from mahendra-rk

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@anuragpatro @wonderslug @codecov-commenter @mahendra-rk