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

Add IBM Cloud Databases list and backups list fetchers #44

Closed

Conversation

Parthakumar-Roy
Copy link

@Parthakumar-Roy Parthakumar-Roy commented Jan 11, 2021

What

Migrate IBM Cloud Databases fetcher for account specific list of databases and backups list.

Why

Audits often request evidence to check whether databases are regularly being backed up.

How

REST api calls will be made to the resource controller and IBM Cloud databases end points.

Test

Local testing to be done to verify evidence being created.

fetch_databases.py

 time compliance --fetch --evidence local --notify stdout -C develop.json arboretum.ibm_cloud_databases.fetchers.fetch_databases

INFO: Using locker found in /var/folders/vv/9fgnxb454235bwsgw1wgl6840000gn/T/compliance...

Fetcher Primary Run

.
----------------------------------------------------------------------
Ran 1 test in 6.136s

OK

INFO: Committing changes to local locker in /var/folders/vv/9fgnxb454235bwsgw1wgl6840000gn/T/compliance...

real	0m6.577s
user	0m0.295s
sys	0m0.131s

The evidence file was generated and its contents verified

ls -la /var/folders/vv/9fgnxb454235bwsgw1wgl6840000gn/T/compliance/raw/icd/
total 120
drwxrwxr-x  4 Parthakumar.Roy@ibm.com  staff    128 Jan 14 16:34 .
drwxrwxr-x  3 Parthakumar.Roy@ibm.com  staff     96 Jan 14 16:19 ..
-rw-rw-r--  1 Parthakumar.Roy@ibm.com  staff  54030 Jan 14 16:34 databases_prod_1329217.json
-rw-rw-r--  1 Parthakumar.Roy@ibm.com  staff    164 Jan 14 16:34 index.json

fetch_backups.py

time compliance --fetch --evidence local --notify stdout -C develop.json arboretum.ibm_cloud_databases.fetchers.fetch_backups

INFO: Using locker found in /var/folders/vv/9fgnxb454235bwsgw1wgl6840000gn/T/compliance...

Fetcher Primary Run

.
----------------------------------------------------------------------
Ran 1 test in 19.527s

OK

INFO: Committing changes to local locker in /var/folders/vv/9fgnxb454235bwsgw1wgl6840000gn/T/compliance...

real	0m19.958s
user	0m0.455s
sys	0m0.145s
(arboretum3) Parthakumar.Roy@ibm.com:auditree-arboretum$ ls -al /var/folders/vv/9fgnxb454235bwsgw1wgl6840000gn/T/compliance/raw/icd/
total 80
drwxrwxr-x  5 Parthakumar.Roy@ibm.com  staff    160 Jan 15 09:58 .
drwxrwxr-x  3 Parthakumar.Roy@ibm.com  staff     96 Jan 14 16:19 ..
-rw-rw-r--  1 Parthakumar.Roy@ibm.com  staff      2 Jan 15 09:58 backups_prod_1329217.json
-rw-rw-r--  1 Parthakumar.Roy@ibm.com  staff  29971 Jan 15 09:54 databases_prod_1329217.json
-rw-rw-r--  1 Parthakumar.Roy@ibm.com  staff    327 Jan 15 09:58 index.json

NOTE: For the fetch_backups test to run to completion, certain entries in the database evidence that were getting 404's had to be manually removed.

Context

#43

Sorry, something went wrong.

@Parthakumar-Roy
Copy link
Author

  • picked a more descriptive ibm_cloud_databases for the source code directory, but left it at icd for the locker repo for backward compatibility.
  • as of this writing, the evidence for the list of databases is now going to change to a list of json objects, one per resource_group_id (previously it was the json object itself).
[
{
 "next_url": null,
 "resources": [
   {
     "account_id": "...",
     "allow_cleanup": false,
     "controlled_by": "",
     "created_at": "2018-10-10T15:50:48.744687589Z",
     "created_by": "",
     "crn": "crn:v1:bluemix:public:databases-for-postgresql:us-south:a/...::",
      ...
     "resource_group_id": "...",
      ...
   },
   ...
  ],
 "rows_count": 13
},
{
 ...
},
...
]
  • as of this writing, the evidence for the list of backups is still a list of the backup objects, no attempt has been made to make it a dictionary of backups list separated by resource_group_id.
[
 {
   "created_at": "2020-11-30T23:31:19.000Z",
   "deployment": {
     "id": "crn:v1:bluemix:public:databases-for-postgresql:us-south:a/...::",
     "version": "10"
   },
   "deployment_id": "crn:v1:bluemix:public:databases-for-postgresql:us-south:a/...::",
   "id": "crn:v1:bluemix:public:databases-for-postgresql:us-south:a/...:backup:...",
   "is_downloadable": false,
   "is_restorable": true,
   "status": "completed",
   "type": "scheduled"
 },
 ...
]
  • followed the fetch_cluster_list approach on how to remove dependency on ibmcloud_tools

Copy link
Contributor

@alfinkel alfinkel left a comment

Choose a reason for hiding this comment

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

Pausing...

It looks like you may be adding functionality, so it's not a straight up migration of these fetchers. If that's what you're doing then your README write up needs to be accurate as to this new behavior for these fetchers. Can you clean up the README and then we can resume review?

Thanks.

Comment on lines 5 to 6
compliance reports and notifications using the [auditree-framework][]. They
validate the configuration and ensure smooth execution of an auditree instance.
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this sentence: They validate the configuration and ensure smooth execution of an auditree instance.

@@ -0,0 +1,120 @@
# -*- coding:utf-8; mode:python -*-
Copy link
Contributor

Choose a reason for hiding this comment

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

You're missing the copyright header here.

CHANGES.md Outdated
Comment on lines 1 to 3
# 0.8.0 (2021-11-01)

- [ADDED] IBM Cloud Databases list and backups list fetchers added.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# 0.8.0 (2021-11-01)
- [ADDED] IBM Cloud Databases list and backups list fetchers added.
# [0.8.0](https://github.com/ComplianceAsCode/auditree-arboretum/releases/tag/v0.8.0)
- [ADDED] IBM Cloud Databases list fetcher added.
- [ADDED] IBM Cloud Databases backups fetcher added.
- [ADDED] Folder hierarchy for IBM Cloud Databases fetchers, checks, and harvest reports added.


* Class: [DatabasesFetcher][fetch-databases]
* Purpose: Store list of account-specific IBM Cloud databases to the evidence locker.
* Behavior: Access [IBM Cloud API][ibm-cloud-api] details of the databases as a JSON. TTL for evidence is set to 1 day.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Behavior: Access [IBM Cloud API][ibm-cloud-api] details of the databases as a JSON. TTL for evidence is set to 1 day.
* Behavior: Retrieve details about account specific IBM Cloud Databases. TTL for evidence is set to 1 day.

* Configuration elements:
* `org.icd.list.accounts`
* Required
* List of objects representing the IBL Cloud accounts
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* List of objects representing the IBL Cloud accounts
* List of dictionaries representing the IBM Cloud accounts

Comment on lines 32 to 39
* `resource_group_id` - the resource group associated with the above account name, output of [running the ibmcloud resource groups command][ic-resource-groups] while logged in to an IBM Cloud account.
```
$ ibmcloud resource groups
Retrieving all resource groups under account fedcba1234567890abcdef0123456789 as dummy.user@ibm.com...
OK
Name ID Default Group State
default bcdef0123456789fedcba1234567890a true ACTIVE
```
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd remove the example and simplify the bullet content. You already provide the link to the command.

Suggested change
* `resource_group_id` - the resource group associated with the above account name, output of [running the ibmcloud resource groups command][ic-resource-groups] while logged in to an IBM Cloud account.
```
$ ibmcloud resource groups
Retrieving all resource groups under account fedcba1234567890abcdef0123456789 as dummy.user@ibm.com...
OK
Name ID Default Group State
default bcdef0123456789fedcba1234567890a true ACTIVE
```
* `resource_group_id` - the resource group associated with the account.
* To obtain the resource group execute the [IBM Cloud resource groups command][ic-resource-groups].

* Required
* List of objects representing the IBL Cloud accounts
* Each object must have the following fields
* `account_name` - an arbitrary name identifying the IBM Cloud account, used to match to the token provided in the
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* `account_name` - an arbitrary name identifying the IBM Cloud account, used to match to the token provided in the
* `account_name` - an arbitrary name as a string identifying the IBM Cloud account, used to match to the token provided in the

* Each object must have the following fields
* `account_name` - an arbitrary name identifying the IBM Cloud account, used to match to the token provided in the
credentials file in order for the fetcher to retrieve content from IBM Cloud for that account.
* `resource_group_id` - the resource group associated with the above account name, output of [running the ibmcloud resource groups command][ic-resource-groups] while logged in to an IBM Cloud account.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* `resource_group_id` - the resource group associated with the above account name, output of [running the ibmcloud resource groups command][ic-resource-groups] while logged in to an IBM Cloud account.
* `resource_group_id` - the resource group as a string associated with the above account name, output of [running the ibmcloud resource groups command][ic-resource-groups] while logged in to an IBM Cloud account.

Comment on lines 51 to 46
"resource_group_id": [
"bcdef0123456789fedcba1234567890a",
...
]
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't valid JSON and renders as such in GH. This example also has resource_group_id as a list of strings whereas the description of it is that it should be a string. Assuming string...

Suggested change
"resource_group_id": [
"bcdef0123456789fedcba1234567890a",
...
]
"resource_group_id": "bcdef0123456789fedcba1234567890a"

Comment on lines 63 to 80
The legacy config format where each account has a single resource_group_id is also supported

```json
{
"org": {
"icd": {
"list": {
"accounts": [
{
"account_name": "prod_1234567",
"resource_group_id": "bcdef0123456789fedcba1234567890a"
}
]
}
}
}
}
```
Copy link
Contributor

Choose a reason for hiding this comment

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

OK now it seems like you're introducing new functionality here? If you're looking to support this new functionality and deprecate the old one where you have a single resource group as a string that's fine but you don't need to include the "backward compatibility" version in the docs.

@Parthakumar-Roy
Copy link
Author

Local test results added to the top - it raises the question of whether 404 errors should abort creation of the backups evidence.

@alfinkel
Copy link
Contributor

What's the question exactly?

@Parthakumar-Roy
Copy link
Author

What's the question exactly?

When fetch_backups.py iterates through a list of databases, if it gets a 404 from any of the backup URL's https://api.<region>.databases.cloud.ibm.com/v4/ibm/deployments/<database_crn_escaped>/backups, the fetcher aborts. Is this acceptable behavior?

@alfinkel
Copy link
Contributor

alfinkel commented Jun 7, 2021

Closing due to inactivity

@alfinkel alfinkel closed this Jun 7, 2021
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

2 participants