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

cluster list fetchers #25

Merged
merged 1 commit into from
Sep 8, 2020
Merged

cluster list fetchers #25

merged 1 commit into from
Sep 8, 2020

Conversation

tmishina
Copy link
Contributor

PR #15 is split into two PRs; cluster list fetchers are included in this PR, and cluster resource fetchers will be included in another PR.

What

This pull request provides a feature to fetch list of kubernetes clusters per account (see #9 in details).
For vanilla kubernetes clusters, a BOM (Bill of Materials) described in a config file will be stored into an evidence locker. For other types of clusters, a list of clusters is fetched by invoking API of each cloud service provider. This PR includes a fetcher for IBM Cloud.

Why

The resources in a kubernetes cluster contain various types of evidences; for example, spec of Pods represents configuration of applications, ConfigMap contains the configuration for the kubernetes cluster itself, and therefore fetching the resources of a kubernetes cluster is important capability for compliance evidence validation of kubernetes clusters.

To fetch resources from a kubernetes cluster, a list of the target clusters is required. This PR includes cluster list fetchers, and its output will be used by cluster resource fetchers (will be provided in another future PR).

How

  • cluster list fetcher: store the list of clusters

Test

  • tests of cluster list fetcher (kubernetes and ibm_cloud) against the IBM Cloud clusters (both IKS and ROKS)
    were passed

Context

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.

  • I didn't review the READMEs. Saving that for later.
  • You need to add unit tests for the iam_ibm module.

arboretum/common/iam_ibm.py Outdated Show resolved Hide resolved
arboretum/common/iam_ibm.py Outdated Show resolved Hide resolved
arboretum/ibm_cloud/fetchers/fetch_cluster_list.py Outdated Show resolved Hide resolved
arboretum/kubernetes/fetchers/fetch_cluster_list.py Outdated Show resolved Hide resolved
@tmishina
Copy link
Contributor Author

tmishina commented Sep 1, 2020

@alfinkel I added a unit test for iam_ibm.py, but it always fails when it is executed on github because required credential (API key) is not stored in the repo. Setting up a test-purpose cluster and generating an API key for the cluster is not an easy way...

Test in a local environment is passed as follows.

 $ python -m unittest test.test_iam_ibm
..
----------------------------------------------------------------------
Ran 2 tests in 1.119s

OK

Is there any solution for this issue?

@drsm79
Copy link
Contributor

drsm79 commented Sep 1, 2020

@alfinkel I added a unit test for iam_ibm.py, but it always fails when it is executed on github because required credential (API key) is not stored in the repo. Setting up a test-purpose cluster and generating an API key for the cluster is not an easy way...

Test in a local environment is passed as follows.

 $ python -m unittest test.test_iam_ibm
..
----------------------------------------------------------------------
Ran 2 tests in 1.119s

OK

Is there any solution for this issue?

Could that be mocked @tmishina ?

@tmishina
Copy link
Contributor Author

tmishina commented Sep 1, 2020

@drsm79

Could that be mocked @tmishina ?

Does "mock" mean an http server process which returns as below?

  • correct tokens when get_tokens() send an http get request with API-key-like string in its header
  • some error (e.g., 404) when get_tokens() send wrong request (such as empty-string API key)

@drsm79
Copy link
Contributor

drsm79 commented Sep 1, 2020

Does "mock" mean an http server process which returns as below?

* correct tokens when `get_tokens()` send an http get request with API-key-like string in its header

* some error (e.g., 404) when `get_tokens()` send wrong request (such as empty-string API key)

https://docs.python.org/3/library/unittest.mock.html you know the good/bad conditions, so should be able to mock the behviour.Not perfect, but gets around needing a live IAM to talk to, and can be extended in the future if responses change.

@tmishina
Copy link
Contributor Author

tmishina commented Sep 1, 2020

@drsm79 thank you, I realized that my knowledge about Python should be update to Python3...l will update the code with unittest.mock.

@tmishina tmishina force-pushed the 9-cluster-list-fetcher branch 2 times, most recently from 259553c to ad7542f Compare September 1, 2020 15:30
@tmishina
Copy link
Contributor Author

tmishina commented Sep 1, 2020

updated with using unittest.mock
https://github.com/ComplianceAsCode/auditree-arboretum/pull/25/files#diff-2de61972d474998cd443ad1de046a017

Now the test successfully finishes in the status checks.

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.

We're getting closer @tmishina...

A few code tweaks, a rewrite of the unit tests, and you still need to remove the BOM fetcher and docs. See: https://github.com/ComplianceAsCode/auditree-arboretum/pull/25/files#r481124180

arboretum/common/iam_ibm.py Outdated Show resolved Hide resolved
arboretum/common/iam_ibm.py Outdated Show resolved Hide resolved
arboretum/ibm_cloud/fetchers/fetch_cluster_list.py Outdated Show resolved Hide resolved
test/test_iam_ibm.py Outdated Show resolved Hide resolved
test/test_iam_ibm.py Outdated Show resolved Hide resolved
@tmishina
Copy link
Contributor Author

tmishina commented Sep 3, 2020

@alfinkel update the usage of mock; does this usage of mock make sense for you?

Other changes include applyin your suggestions and deleting kubernetes/fetch_cluster_list.py which can be substituted by ComplianceConfigFetcher. Updating README.md would be required in addition to the deletion, but it has not been done yet.

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.

I copied this iam_ibm.py and changed it to iam_ibm_utils.py with a few small modifications and submitted it for PR #26. I also added tests for get_tokens. Once #26 is merged you can refresh your branch and retest the fetcher to make sure it still works and just delete your iam_ibm.py and test_iam_ibm.py.

def setUpClass(cls):
"""Initialize the fetcher object with configuration settings."""
headers = {'Accept': 'application/json'}
cls.session('https://containers.cloud.ibm.com', **headers)
Copy link
Contributor

Choose a reason for hiding this comment

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

Move 'https://containers.cloud.ibm.com' to ibm_constants.py (in #26) and use the constant value here instead. Suggest:

# IBM Cloud containers API base URL
IC_CONTAINERS_BASE_URL = 'https://containers.cloud.ibm.com'

after IAM_API_KEY_GRANT_TYPE in constants.py.

Comment on lines 20 to 56
### Cluster List

* Class: [ClusterListFetcher][fetch-cluster-list]
* Purpose: Write the list of kubernetes clusters to the evidence locker.
* Behavior: Read BOM (Bill of Materials) data in config file and write it into evidence locker.
* Configuration elements:
* `org.kubernetes.cluster_list.bom`
* Required
* List where each element contains `account`, `name`, `kubeconfig` and `type`. `kubeconfig` is a path to a kubeconfig file for the cluster. Useer can specify any value for other fields to distinguish the cluster from other clusters.
* Example configuration:

```json
{
"org": {
"kubernetes": {
"cluster_list": {
"bom": [
{
"account": "myaccount",
"name": "mycluster-free",
"kubeconfig": "/home/myaccount/.kube/mycluster-free.kubeconfig",
"type": "kubernetes"
}
]
}
}
}
}
```

* Required credentials:
* A valid kubeconfig file must exist at the path specified in `org.kubernetes.cluster_list.bom[].kubeconfig`.
* Import statement:

```python
from arboretum.kubernetes.fetchers.fetch_cluster_list import ClusterListFetcher
```
Copy link
Contributor

Choose a reason for hiding this comment

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

Should return this to original Fetchers coming soon...

Comment on lines 1 to 39
# -*- mode:python; coding:utf-8 -*-
# Copyright (c) 2020 IBM Corp. All rights reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
"""Utility module for IBM Cloud IAM."""
import requests


def get_tokens(api_key):
"""
Get tokens (access token and refresh token) by api_key.

see https://cloud.ibm.com/apidocs/iam-identity-token-api
"""
headers = {
'Content-Type': 'application/x-www-form-urlencoded',
'Accept': 'application/json'
}
grant_type = 'urn:ibm:params:oauth:grant-type:apikey'
resp = requests.post(
'https://iam.cloud.ibm.com/identity/token',
headers=headers,
auth=('bx', 'bx'),
data=f'grant_type={grant_type}&apikey={api_key}'
)
resp.raise_for_status()
access_token = resp.json()['access_token']
refresh_token = resp.json()['refresh_token']
return access_token, refresh_token
Copy link
Contributor

Choose a reason for hiding this comment

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

Replace this with iam_ibm_utils.py from #26 once it is merged.

Comment on lines 1 to 87
# -*- mode:python; coding:utf-8 -*-
# Copyright (c) 2020 IBM Corp. All rights reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
"""Arboretim IBM Cloud IAM utitity module tests."""
import unittest
from unittest import mock

from arboretum.common.iam_ibm import get_tokens

from requests import HTTPError, Response


def _mock_requests_post(*args, **kwargs):
data = kwargs['data']
kw = 'apikey='
idx = data.find(kw)
api_key = data[idx + len(kw):]

def json_function(**kwargs):
return {
'access_token': 'yI6ICJodHRwOi8vc2VydmVyLmV4YW1y'
'I6ICJodHRwOi8vc2VydmVyLmV4YW1',
'refresh_token': 'yI6ICJodHRwOi8vc2VydmVyLmV4YW1y'
'I6ICJodHRwOi8vc2VydmVyLmV4YW1'
}

resp = Response()
if len(api_key) > len('yI6ICJodHRwOi8vc2VydmVyLmV4YW1'):
resp.status_code = 200
resp.json = json_function
else:
raise HTTPError() # status_code is automatically set to 400.
return resp


class IamIbmTest(unittest.TestCase):
"""Arboretum IBM Cloud IAM functions tests."""

@mock.patch('requests.post')
def test_get_tokens_success(self, mock_post):
"""Ensure that an API call with valid API key returns valid tokens."""
resp = Response()
resp.json = lambda **kwargs: {
'access_token': 'yI6ICJodHRwOi8vc2VydmVyLmV4YW1y'
'I6ICJodHRwOi8vc2VydmVyLmV4YW1',
'refresh_token': 'yI6ICJodHRwOi8vc2VydmVyLmV4YW1y'
'I6ICJodHRwOi8vc2VydmVyLmV4YW1'
}
resp.status_code = 200
mock_post.return_value = resp
api_key = 'yI6ICJodHRwOi8vc2VydmVyLmV4YW1xxxx'
access_token, refresh_token = get_tokens(api_key)

# spec of access_token and refresh_token
# https://cloud.ibm.com/docs/mobilefoundation?topic=mobilefoundation-basic_authentication
# the doc above says tokens are longer than its examples.
self.assertIsInstance(access_token, str)
self.assertGreater(
len(access_token), len('yI6ICJodHRwOi8vc2VydmVyLmV4YW1')
)
self.assertIsInstance(refresh_token, str)
self.assertGreater(
len(refresh_token), len('yI7ICasdsdJodHRwOi8vc2Vashnneh')
)

# ensure requests.post() is called as expected
mock_post.assert_called_once_with(
'https://iam.cloud.ibm.com/identity/token',
headers={
'Content-Type': 'application/x-www-form-urlencoded',
'Accept': 'application/json'
},
auth=('bx', 'bx'),
data='grant_type=urn:ibm:params:oauth:grant-type:'
f'apikey&apikey={api_key}'
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Delete this file and just use what's in #26

@alfinkel
Copy link
Contributor

alfinkel commented Sep 3, 2020

@tmishina #26 is merged into main so you're good to go with applying changes from the above review.

@tmishina
Copy link
Contributor Author

tmishina commented Sep 4, 2020

@alfinkel applied your comments and perform git rebase -i. Please tell me if further tasks that I need to do remain.

@tmishina
Copy link
Contributor Author

tmishina commented Sep 6, 2020

@alfinkel Thank you, I applied your suggestion with additional import statements. Also, I performed git rebase -i again and fixup commits into single commit.

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.

+1 - I'll merge and then add a follow up PR to update the missing CHANGES.md entry and version bump and then make a release.

@alfinkel alfinkel merged commit 2ec0137 into main Sep 8, 2020
@alfinkel alfinkel deleted the 9-cluster-list-fetcher branch September 8, 2020 13:33
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