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

Fixes #119 #123

Merged
merged 7 commits into from
Dec 30, 2016
Merged

Fixes #119 #123

merged 7 commits into from
Dec 30, 2016

Conversation

wojtek0806
Copy link
Contributor

Problem:

_validate_suffix_collections method assumes that '~' and '/' in elements are incorrect. This leads to issues where 'name' contains '~' or '/' on purpose.

Analysis:
This patch introduces name_transform bool (with default being False), with it set to True, the 'name' element will not be validated for the presence of '~' and '/' characters. It will also convert all '/' in the name into '~'. To use this ability BIGIP needs to pass the argument 'name_transform' in the **kwargs when calling the iControl.

Tests:
unit

@wojtek0806
Copy link
Contributor Author

def transform_name_w_subpath():
parts_dict = {'base_uri': 'https://0.0.0.0/mgmt/tm/root/RESTiface/',
'partition': 'BIGCUSTOMER',
'name': 'foobar1: 1.1.1.1/24 bar1: /Common/DC1',
Copy link
Contributor

@pjbreaux pjbreaux Nov 23, 2016

Choose a reason for hiding this comment

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

Is this a valid name on the BIG-IP?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep, as discussed this is a valid name for gtm topology records

@pjbreaux
Copy link
Contributor

I'd like to see a set of functional tests as well with this. Perhaps creating a partition/folder with a name that has slashes? Things like that.

@wojtek0806
Copy link
Contributor Author

ill create and try to load the record as discussed on slack

@wojtek0806
Copy link
Contributor Author

@pjbreaux added functional tests feel free to test on GTM enabled 11.5.4 unit and merge when ready :)

Problem:

_validate_suffix_collections method assumes that '~' and '/' in elements are incorrect. This leads to issues where 'name' contains '~' or '/' on purpose.

Analysis:
This patch introduces name_transform bool (with default being False), with it set to True, the 'name' element will not be validated for the presence of '~' and '/' characters. It will also convert all '/' in the name into '~'. To use this ability BIGIP needs to pass the argument 'name_transform' in the **kwargs when calling the iControl.

Tests:
unit
functional
@pjbreaux
Copy link
Contributor

@wojtek0806: Could you post your test results here. Then I can merge.

Thanks

@pjbreaux
Copy link
Contributor

We'll need to paste the results of all the tests, ensuring no regressions occurred.

@wojtek0806
Copy link
Contributor Author

https://gist.github.com/wojtek0806/5f41b0b695e9084b56b7e77a62139112

The tests for auth fail cause i am running these against 11.5.4, those are unrelated to the changes I have
made

@pjbreaux

@pjbreaux
Copy link
Contributor

I don't see all of the tests being run here, just the auth failures. I think we still should test against a version where the auth tests succeed, just to ensure we haven't made any changes that haven't broken auth. And to get another data point of testing against a new device version.

@wojtek0806
Copy link
Contributor Author

i am sorry but if you want to test this test it, please provide a jerkins like solution here, or you can pull my fork run the tests yourself, also my tests are aimed at 11.5.4 and 11.6.0 specifically due to how buggy GTM topology is.

@pjbreaux
Copy link
Contributor

Running it against 11.5.4 and 11.6.0 sounds fine. For now it is manual, but I agree, it must be a Jenkins job or part of a build.

@wojtek0806
Copy link
Contributor Author

@pjbreaux I have asked @caphrim007 to attach this to Jenkins, then I can rework the entire testing suite we have so far for functional tests and we can run this cross versions and do what we do in f5-sdk. I hate manual tasks with passion :)

@wojtek0806
Copy link
Contributor Author

See attached tests, those are exports from Pycharm,

11.5.4 Fails Auth tests due to BIGIP sending us 404 response, could be because technically tokens were not implemented in 11.5.x but this is just a guess. 11.6.0 tests pass "test_get_special_name_11_x_12_0" and "test_get_special_name_12_1" since it handles the extra spaces in resource names.

Tests test_get_special_name_11_x_12_0 pass in all versions but 12.1.0
Version 12.0.0 has a bug that prevents us from creating objects in the first place.

Test Results - all.zip

@pjbreaux

@pjbreaux
Copy link
Contributor

Could you skip the appropriate tests and just mark up with a message as to why the auth tests fail against 11.5.4 and why the special name test fails in 12.1.

@pjbreaux
Copy link
Contributor

After that, I can merge it in.

@wojtek0806
Copy link
Contributor Author

hmm i am not sure how do i do that, i only run pytest on pycharm, so what exactly do you wanna do? just pytest.skip ?

@pjbreaux
Copy link
Contributor

Yes, just the skips and a message along with it.

@pjbreaux pjbreaux merged commit 4051936 into F5Networks:1.0 Dec 30, 2016
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.

3 participants