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

secret management #104

Merged
merged 6 commits into from
Sep 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
131 changes: 131 additions & 0 deletions maap/Secrets.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
import requests
import logging
import json
from maap.utils import endpoints
from maap.utils import requests_utils
from maap.utils import endpoints

logger = logging.getLogger(__name__)

class Secrets:
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add a section in the functional test to test adding, retrieving and deleting a secret?
https://github.com/MAAP-Project/maap-py/blob/master/test/functional_test.py

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, this is next on my to-do.

Copy link
Member Author

Choose a reason for hiding this comment

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

Implemented.

"""
Functions used for member secrets API interfacing
"""
def __init__(self, member_endpoint, api_header):
self._api_header = api_header
self._members_endpoint = f"{member_endpoint}/{endpoints.MEMBERS_SECRETS}"


def get_secrets(self):
"""
Returns a list of secrets for a given user.

Returns:
list: Returns a list of dicts containing secret names e.g. [{'secret_name': 'secret1'}, {'secret_name': 'secret2'}].
"""
try:
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
try:
try:
logger.debug("Fetching all secrets")

Just more informative to log what function is being called than just logging response.text.

response = requests.get(
url = self._members_endpoint,
headers=self._api_header
)
logger.debug(f"Response from get_secrets request: {response.text}")
return json.loads(response.text)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be of type list or dict to match the doc string?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a list of dicts. I've updated the doc string to provide an example.

except Exception as e:
raise(f"Error retrieving secrets: {e}")


def get_secret(self, secret_name):
"""
Returns secret value for provided secret name.

Args:
secret_name (str, required): Secret name.

Returns:
string: Secret value.

Raises:
ValueError: If secret name is not provided.
"""
if secret_name is None:
raise ValueError("Secret name parameter cannot be None.")

try:
response = requests.get(
url = f"{self._members_endpoint}/{secret_name}",
headers=self._api_header
)

# Return secret value directly for user ease-of-use
if response.ok:
response = response.json()
return response["secret_value"]

logger.debug(f"Response from get_secret request: {response.text}")
return json.loads(response.text)
except Exception as e:
raise(f"Error retrieving secret: {e}")


def add_secret(self, secret_name=None, secret_value=None):
"""
Adds a secret. Secret name must be provided. Secret value may be null.
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the use-case of having a secret without a value ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Mostly because an empty string is a valid value and I don't see a good reason placing additional constraints on this. I was also thinking of cases where users might need to have the secret parameter exist but not necessarily care about the value itself.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Saw this later, commented below.


Args:
secret_name (str, required): Secret name.
secret_value (str, optional): Secret value.

Returns:
dict: Containing name and value of secret that was just added.

Raises:
ValueError: If secret name or secret value is not provided.
"""
if secret_name is None or secret_value is None:
raise ValueError("Failed to add secret. Secret name and secret value must not be 'None'.")

try:
response = requests.post(
url = self._members_endpoint,
headers=self._api_header,
data=json.dumps({"secret_name": secret_name, "secret_value": secret_value})
)

logger.debug(f"Response from add_secret: {response.text}")
return json.loads(response.text)
except Exception as e:
raise(f"Error adding secret: {e}")


def delete_secret(self, secret_name=None):
"""
Deletes a secret.

Args:
secret_name (str, required): Secret name.

Returns:
dict: Containing response code and message indicating whether or not deletion was successful.

Raises:
ValueError: If secret name is not provided.
"""
if secret_name is None:
raise ValueError("Failed to delete secret. Please provide secret name.")

try:
response = requests.delete(
url = f"{self._members_endpoint}/{secret_name}",
headers=self._api_header
)

logger.debug(f"Response from delete_secret: {response.text}")
return json.loads(response.text)
except Exception as e:
raise(f"Error deleting secret: {e}")






4 changes: 3 additions & 1 deletion maap/maap.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from maap.utils import algorithm_utils
from maap.Profile import Profile
from maap.AWS import AWS
from maap.Secrets import Secrets
from maap.dps.DpsHelper import DpsHelper
from maap.utils import endpoints

Expand All @@ -40,10 +41,11 @@ def __init__(self, maap_host=os.getenv('MAAP_API_HOST', 'api.maap-project.org'))
self.config.workspace_bucket_credentials,
self._get_api_header()
)
self.secrets = Secrets(self.config.member, self._get_api_header(content_type="application/json"))

def _get_api_header(self, content_type=None):

api_header = {'Accept': content_type if content_type else self.config.content_type, 'token': self.config.maap_token}
api_header = {'Accept': content_type if content_type else self.config.content_type, 'token': self.config.maap_token, 'Content-Type': content_type if content_type else self.config.content_type}

if os.environ.get("MAAP_PGT"):
api_header['proxy-ticket'] = os.environ.get("MAAP_PGT")
Expand Down
2 changes: 2 additions & 0 deletions maap/utils/endpoints.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,5 @@
DPS_JOB_DISMISS = "cancel"
DPS_JOB_LIST = "list"
CMR_ALGORITHM_DATA = "data"

MEMBERS_SECRETS = "secrets"
35 changes: 35 additions & 0 deletions test/functional_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,31 @@ def cancel_job(maap: MAAP, job_id):
assert resp is not None
assert 'Accepted' in str(resp)

@log_decorator
def add_secret(maap: MAAP, secret_name=None, secret_value=None):
resp = maap.secrets.add_secret(secret_name, secret_value)
print(resp)
assert resp is not None

@log_decorator
def get_secrets(maap: MAAP):
resp = maap.secrets.get_secrets()
print(resp)
assert resp is not None

@log_decorator
def get_secret(maap: MAAP, secret_name=None, secret_value=None):
resp = maap.secrets.get_secret(secret_name)
print(resp)
assert resp is not None
assert resp == secret_value

@log_decorator
def delete_secret(maap: MAAP, secret_name=None):
resp = maap.secrets.delete_secret(secret_name)
print(resp)
assert resp is not None


def main():
if os.environ.get('MAAP_PGT') is None:
Expand All @@ -118,9 +143,19 @@ def main():
# list_algorithms(maap)
job = submit_job(maap, queue="maap-dps-sandbox")
cancel_job(maap, job.id)

# Test secrets management
secret_name = "test_secret"
secret_value = "test_value"
get_secrets(maap)
add_secret(maap, secret_name, secret_value)
get_secret(maap, secret_name, secret_value)
delete_secret(maap, secret_name)

# submit_job(maap, wait_for_completion=True)
# delete_algorithm(maap, "maap_functional_test_algo:main")



if __name__ == '__main__':
main()
Loading