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

Fix bugs with moving account & ou and filtering for organization endpoint. #2168

Merged
merged 5 commits into from
May 28, 2020

Conversation

myersCody
Copy link
Contributor

Jira Issues:
https://issues.redhat.com/browse/COST-160
https://issues.redhat.com/browse/COST-157

This pr fixes some bugs in the organizational endpoint.

COST-160

This bug is because I didn't catch accounts and sub_ou moving from one organizational unit (ou) to another ou properly. Therefore, the account was showing up in its previous ou and the new ou. The same goes for child organizational unit moving from one parent to another.

How to test

Therefore to test that this has been fixed you will need to bring up the server and run make load-aws-org-unit-tree

After the tree has been populated in the database, we can look at this endpoint: http://127.0.0.1:8000/api/cost-management/v1/organizations/aws/

Check Moving Account

The account 003 was moved from OU_002 to OU_003. Therefore, we just need to check that account 003 no longer shows up in OU_002, but does show up in OU_003

Check Moving OU

The child organizational unit or sub_ou OU_005 was moved from OU_002 to OU_001. Therefore, we just need to check that OU_005 is not in OU_OO2's sub_orgs but is in OU_001

Note: You can check how the accounts and ou are moved by looking at the aws_org_tree.yml file.

COST-157

This bug was that the sub_orgs were not showing up when using ?filter[org_unit_id]=OU_001. I managed to fix this as a result of fixing 160. To test this one you just need to make sure the sub_orgs now show up when you hit the endpoint with the filter. If you already have run make load-aws-org-unit-tree you should be able to go to: http://127.0.0.1:8000/api/cost-management/v1/organizations/aws/?filter[org_unit_id]=OU_001 and see:

        {
            "org_unit_id": "OU_001",
            "org_unit_name": "Dept OU_001",
            "org_unit_path": "R_001&OU_001",
            "level": 1,
            "sub_orgs": [
                "OU_005"
            ],
            "accounts": [
                "account 001",
                "account 002"
            ]
        }

@myersCody myersCody added the bug Something isn't working label May 27, 2020
@myersCody myersCody requested a review from a team May 27, 2020 22:27
@myersCody myersCody self-assigned this May 27, 2020
@codecov
Copy link

codecov bot commented May 27, 2020

Codecov Report

Merging #2168 into master will increase coverage by 0.0%.
The diff coverage is 100.0%.

@@          Coverage Diff           @@
##           master   #2168   +/-   ##
======================================
  Coverage    94.4%   94.4%           
======================================
  Files         230     230           
  Lines       15959   15963    +4     
  Branches     1821    1821           
======================================
+ Hits        15061   15067    +6     
+ Misses        589     587    -2     
  Partials      309     309           

Copy link
Contributor

@abaiken abaiken left a comment

Choose a reason for hiding this comment

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

I will re-review as soon as RBAC is fixed! 😄

koku/api/organizations/queries.py Show resolved Hide resolved
Copy link
Contributor

@abaiken abaiken left a comment

Choose a reason for hiding this comment

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

LGTM & I tested it 👍 We should hold off merging this until the CI dependency issues are resolved.

@myersCody myersCody merged commit 50004fc into master May 28, 2020
@myersCody myersCody deleted the jira-160_and_157 branch May 28, 2020 19:32
lcouzens pushed a commit that referenced this pull request Jun 1, 2020
* 1912 tags endpoint (#2132)

* validate the key, return 404, return only the values

* convert url to path

* exclude key_only query

* update openspec

* github-action workflow/pre-commit config updates (#2141)

* bump setup-python version. specify py36.

* specify py36 in pre-commit-config

* fix f541 errors

* add some defensive coding for issues found by Sentry (#2142)

* add some defensive coding for issues found by Sentry

* Delete SIUnitScale Model (#2077)

* Delete SIUnitScale Model

* fix migration conflict

Co-authored-by: Michael Skarbek <mskarbek@redhat.com>

* Feature/#2004 - Convert Cloud Accounts JSON to Python (#2074)

* move cloud-accounts json to python constant.

* cleanup some metrics view logic.

* create a simple ListPaginator

Co-authored-by: Michael Skarbek <mskarbek@redhat.com>

* Add guard when no Azure container is provided. (#2144)

* Resiliency and large OCP payload support for listener (#2134)

* add django-extensions, remove drf-nested-routers (#2145)

* add django-extensions, remove drf-nested-routers
* update check-manifest to recommend using the more comprehensive 'make requirements' command.

* make cost mgmt message filter run in thread pool (#2147)

* Validating source cost-model associations for cost-model creation/update (#2146)

* Sources: Concurrent cleanup (#2148)

* remove ThreadPoolExecutor from process_message. move to listen_for_messages.

* remove cost_mgmt_msg_filter from executor

* Improve Masu AWS report ingest logging clarity (#2121)

Signed-off-by: Boaz Shuster <boaz.shuster.github@gmail.com>

Co-authored-by: Andrew Berglund <aberglun@redhat.com>
Co-authored-by: Brett Lentz <blentz@users.noreply.github.com>

* Add debug messages to reslove sentry ErrorDetail has no pop (#2152)

Signed-off-by: Boaz Shuster <boaz.shuster.github@gmail.com>

* Remove logging source credential. (#2153)

* Change info log entry to remove credential logging.

* Fix cast comparison issue in autovacuum task. (#2154)

* Add cast to Decimal

* Revert "Sources: Concurrent cleanup (#2148)" (#2158)

This reverts commit 8fe60ad.

* AWS org unit feature  (#2139)

* Create an aws account hierarchy crawler  (#2023)

* Created an aws account hierarchy crawler

Co-authored-by: Kevan Holdaway <kholdawa@redhat.com>

* Organizations api  (#2062)

* Add AWS organizations API
* Got the time filtering working & added pagination.
* Improve test coverage.
* Added some more tests and cleaned up the serializer.
* Refactor the orgaznizations api to be like the tags api.
* I got the key_only functionality working, and did some cleanup.
* Update tests and fix serializer.

* WIP: 1915 correlate org unit (#2049)

* Model updates for org unit correlation
* Add org unit matching to AWS summary sql
Co-authored-by: Kevan Holdaway <kholdawa@redhat.com>

* Model updates for delete

* Rename aws org unit migrations

* Partial work on creating org structure for a time

* Fix the date range stuff for the org unit api.

* Added sub_orgs to the api data return for org units.

* Update test for tree node and other cleanup

* Green path for getting the sub_orgs through level instead of parentId.

* Got the green path for account alias foreign key in org unit model working.

* Remove account id from models

* Update crawler to use account_alias foreign key

* The org api is now using the quries to return the list.

* Tree delete first pass (updates/tests coming)

* Fix the masu unittests to be working.

* I got group_by org_id working for the organizations api.

* Updates to fix crawler delete function

* Fixed the sql for the org unit foreign key and fix some tests.

* Updates to crawler

* Fix some of the unittests for the org unit api.

* initial changes to add org_unit group_by and filter to aws reports

* Adding script for aws org tree.

* Restructure script to be used for both makefile and unittest workflow.

* Restructure the yml file and insert aws org tree script.

* add initial changes to get the tests to use the insert_org_tree script

* update the account ids so that the data matches and the foreign key is populated.

* use the same range start as the nisedataloader

* Fix test due to koku test runner changes.

* Improve test for crawler.

* add query tests and update aws_org_tree to have org unit 5

* combine migrations

* fix lint errors

* Fix pylint.

* Update azure_report_downloader.py

fix format message

* remove group_by for storage and instance types, fix the tests, and fix the group_pos for the group_by

* configure access after manipulating group_by and add tests.

* Clean up total_sum function for query handler.

* Apply suggestions from code review

* add the caller to the query params

* Apply suggestions from code review

Co-authored-by: Brett Lentz <blentz@users.noreply.github.com>

* move Validation error to serializers and update tests.

* Add provider map to its own file.

* Change the filter org_id for organizations api and org_unit for aws reports to be org_unit_id

* Apply suggestions from code review

Co-authored-by: Douglas Curtis <docurtis@redhat.com>

* add debug log to see the query table

Co-authored-by: Cody Myers <cmyers@redhat.com>
Co-authored-by: Kevan Holdaway <kholdawa@redhat.com>
Co-authored-by: Andrew Berglund <aberglun@redhat.com>
Co-authored-by: myersCody <codymyers63@gmail.com>
Co-authored-by: Kevan Holdaway <akokdh15@gmail.com>
Co-authored-by: Brett Lentz <blentz@users.noreply.github.com>
Co-authored-by: Douglas Curtis <docurtis@redhat.com>

* Handle invalid source type flows for provider accessor. (#2160)

* Handle invalid source type flows for provider accessor.

* Create a check to raise error if source type was invalid on provider accessor creation
* Refactor error_msg function to common location.

* Update import for common error_obj method.

* Revert revert cleanup (#2162)

* Cost 133 upgrade postgres (#2155)

* Update localenv and pg docker image

* A bit of scope-creep here to remove HC pw

* Make tune-up

* Bump connection limit on DB

* Lock PG to version 12

* Increase code coverage

* Fix bad test

* Update readme

* Update README Issue#2156 (#2161)

* Update README

* Updated README

Co-authored-by: Andrew Berglund <aberglun@redhat.com>

* update load test customer data for nise cli changes (#2157)

* update to Nise 2.0 + other dep updates. adjust Makefile for new Nise args

* Address sources sentry errors (#2166)

* RBAC implementation for AWS org units  (#2163)

* initial changes to add RBAC to reports endpoint for orgs
* add in the organizations api rbac piece
* add check for none
Co-authored-by: Douglas Curtis <docurtis@redhat.com>

* Fix paginated tag values [COST-163] (#2169)

* check for "key" attribute before truncating data list

* Update Pipfile.lock after latest master pull. (#2170)

*Try and resolve dependency issues in OpenShift.

* Add S2I override files for koku (#2173)

* Specify Pipenv version

* Fix bugs with moving account & ou and filtering for organization endpoint. (#2168)

* Update openapi.json to document organizations endpoint  (#2174)

* add initial organizations endpoint and fix tag errors

* add filter & group_by info for org_unit_id

* update the accounts and suborgs

* add 403 response

Co-authored-by: Michael Skarbek <mskarbek@redhat.com>
Co-authored-by: Brett Lentz <blentz@users.noreply.github.com>
Co-authored-by: Nick Bonilla <nbon12@users.noreply.github.com>
Co-authored-by: Douglas Curtis <docurtis@redhat.com>
Co-authored-by: boaz0 <boaz.shuster.github@gmail.com>
Co-authored-by: Andrew Berglund <aberglun@redhat.com>
Co-authored-by: Ashley Aiken <aaiken@redhat.com>
Co-authored-by: Cody Myers <cmyers@redhat.com>
Co-authored-by: Kevan Holdaway <kholdawa@redhat.com>
Co-authored-by: myersCody <codymyers63@gmail.com>
Co-authored-by: Kevan Holdaway <akokdh15@gmail.com>
Co-authored-by: HAP <59652772+Red-HAP@users.noreply.github.com>
Co-authored-by: lrlevine6 <65563974+lrlevine6@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants