Security groups #850
Merged
Security groups #850
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 12 commits
July 28, 2017 09:55
This adds security group API helpers to the network manager. This allows users to perform CRUD operations on security groups, security group rules, and interface attachments to security groups. CLI commands were also added to perform these API operations on the command line. Arguments were also added to the instance creation to allow VSIs to be booted with security groups on the public and/or private interfaces of the newly created VSI. Security group information was added to the VSI detail CLI, so the security groups attached to the interfaces of a VSI are output as part of the detail.
In order to list the security groups attached to a VSI, we need to add security groups to the get_instance mask. This is retrieved via networkComponents[securityGroupBindings[securityGroups]].
When VSIs are not available to view because of permissions, we need to block them out of the CLI when doing a detail or interface-list.
The mask in the get_securitygroup() function in the manager was wrong, so fixing that allows the remoteGroupId to print in securitygroup detail.
Because some of the .get() calls return an empty string, they're not actually getting filled with formatting.blank(). This changes the get() calls to add the or after the get() call. This way, if the returned value is empty string (so not set), we'll fill it with a formatting.blank() instead of filling it with nothing.
Being consistent is a good thing.
|
This looks good to me so far, although I'll want to test it locally myself before merging. |
|
If you try it out, take a look, and find nothing wrong with it, it's ready to merge. I have no further changes I'm looking to make on it. |
|
since this looks to still be in beta I think I will hold off on merging until its released officially for everyone. Otherwise the change looks good and works for me. Change Log
|
|
@allmightyspiff we are now in open Beta .. which means we have publicized this offering. https://twitter.com/IBMBluemix/status/895048755896045569 I think this will be a good time to merge this PR. |
|
PR LGTM |
|
@Neetuj thanks for that confirmation. I'll merge and release this today. |
If a dict is passed in to the add_securitygroup_rules function, it is serialized as an iterable and sent over XMLRPC. This now does a type check to make sure it's a list before sending it to be serialized.
|
I added a type check to one of the manager functions for security groups. This is now ready to go. 👍 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change adds security groups CLIs. It also adds helpers for manipulating security groups to the network manager. Security groups can be added to a VSI at order time, so public and private security groups were added to the ordering helper.