diff --git a/charts/shipyard/values.yaml b/charts/shipyard/values.yaml index 1ea5af98..5ff9477d 100644 --- a/charts/shipyard/values.yaml +++ b/charts/shipyard/values.yaml @@ -372,6 +372,7 @@ conf: workflow_orchestrator:get_renderedconfigdocs: rule:admin_read_access workflow_orchestrator:list_workflows: rule:admin_read_access workflow_orchestrator:get_workflow: rule:admin_read_access + workflow_orchestrator:get_notedetails: rule: admin_read_access workflow_orchestrator:get_site_statuses: rule:admin_read_access workflow_orchestrator:action_deploy_site: rule:admin_create workflow_orchestrator:action_update_site: rule:admin_create diff --git a/doc/source/API.rst b/doc/source/API.rst index 6fa3ee55..130f15b3 100644 --- a/doc/source/API.rst +++ b/doc/source/API.rst @@ -26,6 +26,7 @@ Shipyard functionality: 3. Airflow Monitoring 4. Site Statuses 5. Logs Retrieval +6. Notes Handling Standards used by the API ------------------------- @@ -1205,5 +1206,68 @@ Example [2018-04-11 07:30:43,945] {{base_task_runner.py:98}} INFO - Subtask: [2018-04-11 07:30:43,944] {{python_operator.py:90}} INFO - Done. Returned value was: None [2018-04-11 07:30:43,992] {{base_task_runner.py:98}} INFO - Subtask: """) +Notes Handling API +------------------ +The notes facilities of Shipyard are primarily interwoven in other APIs. This +endpoint adds the ability to retrieve additional information associated with a +note. The first use case for this API is the retrieval of builddata from +Drydock, which can be many hundreds of kilobytes of text. + +/v1.0/notedetails/{note_id} +~~~~~~~~~~~~~~~~~~~~~~~~~~~ +Retrieves the note details that are associated via URL with a note at the time +of note creation. Unlike some responses from Shipyard, this API returns the +remote information as-is, as the response body, without any further wrapping in +a JSON structure. + +Entity Structure +^^^^^^^^^^^^^^^^ +Raw text of the note's associated information. + +GET /v1.0/notedetails/{node_id} +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +Looks up the specified note and follows the associated URL to retrieve +information related to the note. + +Query parameters +'''''''''''''''' + +N/A + +Responses +''''''''' + +200 OK + + Accompanied by the text looked up from the note's associated URL + +400 Bad Request + + When the note_id is not a valid ULID value. + +404 Not Found + + When the note does not exist, or the note does not have a URL associated. + +500 Internal Server Error + + When the remote source of the information cannot be accessed, or if there is + a misconfiguration of the type of note preventing appropriate authorization + checking. + +Example +''''''' + +:: + + curl -D - \ + -X GET $URL/api/v1.0/notedetails/01CASSSZT7CP1F0NKHCAJBCJGR \ + -H "X-Auth-Token:$TOKEN" + + HTTP/1.1 200 OK + x-shipyard-req: 49f74418-22b3-4629-8ddb-259bdfccf2fd + + Potentially a lot of information here + .. _API Conventions: https://airshipit.readthedocs.io/en/latest/api-conventions.html diff --git a/doc/source/CLI.rst b/doc/source/CLI.rst index 853cd17c..b166503b 100644 --- a/doc/source/CLI.rst +++ b/doc/source/CLI.rst @@ -383,6 +383,10 @@ Retrieves the detailed information about the supplied namespaced item Equivalent to: shipyard describe action 01BTG32JW87G0YKA1K29TKNAFX + shipyard describe notedetails/01BTG32JW87G0YKA1KA9TBNA32 + Equivalent to: + shipyard describe notedetails 01BTG32JW87G0YKA1KA9TBNA32 + shipyard describe step/01BTG32JW87G0YKA1K29TKNAFX/preflight Equivalent to: shipyard describe step preflight --action=01BTG32JW87G0YKA1K29TKNAFX @@ -413,7 +417,6 @@ Retrieves the detailed information about the supplied action id. Sample ^^^^^^ - :: $ shipyard describe action/01BZZK07NF04XPC5F4SCTHNPKN @@ -426,7 +429,7 @@ Sample Context Marker: 71d4112e-8b6d-44e8-9617-d9587231ffba User: shipyard - Steps Index State Notes + Steps Index State Footnotes step/01BZZK07NF04XPC5F4SCTHNPKN/action_xcom 1 success step/01BZZK07NF04XPC5F4SCTHNPKN/dag_concurrency_check 2 success step/01BZZK07NF04XPC5F4SCTHNPKN/deckhand_get_design_version 3 failed (1) @@ -435,17 +438,39 @@ Sample step/01BZZK07NF04XPC5F4SCTHNPKN/deckhand_get_design_version 6 failed step/01BZZK07NF04XPC5F4SCTHNPKN/drydock_build 7 None - (1): - - step metadata: deckhand_get_design_version(2017-11-27 20:34:34.443053): Unable to determine version + Step Footnotes Note + (1) > step metadata: deckhand_get_design_version(2017-11-27 20:34:34.443053): Unable to determine version + - Info available with 'describe notedetails/09876543210987654321098765' Commands User Datetime invoke shipyard 2017-11-27 20:34:34.443053+00:00 Validations: None - Notes: - action metadata: 01BZZK07NF04XPC5F4SCTHNPKN(2017-11-27 20:34:24.610604): Invoked using revision 3 + Action Notes: + > action metadata: 01BZZK07NF04XPC5F4SCTHNPKN(2017-11-27 20:34:24.610604): Invoked using revision 3 + +describe notedetails +~~~~~~~~~~~~~~~~~~~~ +Retrieves extended information related to a note. + +:: + + shipyard describe notedetails + + Example: + shipyard describe notedetails/01BTG32JW87G0YKA1KA9TBNA32 + + + The id of the note referenced as having more details in a separate response + +Sample +^^^^^^ + +:: + + $ shipyard describe notedetails/01BTG32JW87G0YKA1KA9TBNA32 + describe step ~~~~~~~~~~~~~ @@ -469,7 +494,6 @@ Retrieves the step details associated with an action and step. Sample ^^^^^^ - :: $ shipyard describe step/01BZZK07NF04XPC5F4SCTHNPKN/action_xcom @@ -483,6 +507,10 @@ Sample Try Number: 1 Operator: PythonOperator + Step Notes: + > step metadata: deckhand_get_design_version(2017-11-27 20:34:34.443053): Unable to determine version + - Info available with 'describe notedetails/09876543210987654321098765' + describe validation ~~~~~~~~~~~~~~~~~~~ @@ -508,7 +536,6 @@ validation id Sample ^^^^^^ - :: TBD @@ -533,7 +560,6 @@ workflow engine. Sample ^^^^^^ - :: $ shipyard describe workflow deploy_site__2017-11-27T20:34:33.000000 @@ -590,17 +616,13 @@ Sample :: $ shipyard get actions - Name Action Lifecycle Execution Time Step Succ/Fail/Oth Notes - deploy_site action/01BTP9T2WCE1PAJR2DWYXG805V Failed 2017-09-23T02:42:12 12/1/3 (1) - update_site action/01BZZKMW60DV2CJZ858QZ93HRS Processing 2017-09-23T04:12:21 6/0/10 (2) + Name Action Lifecycle Execution Time Step Succ/Fail/Oth Footnotes + deploy_site action/01BTP9T2WCE1PAJR2DWYXG805V Failed 2017-09-23T02:42:12 12/1/3 (1) + update_site action/01BZZKMW60DV2CJZ858QZ93HRS Processing 2017-09-23T04:12:21 6/0/10 (2) - (1): - - action metadata:01BTP9T2WCE1PAJR2DWYXG805V(2017-09-23 02:42:23.346534): Invoked with revision 3 - - (2): - - action metadata:01BZZKMW60DV2CJZ858QZ93HRS(2017-09-23 04:12:31.465342): Invoked with revision 4 + Action Footnotes Note + (1) > action metadata:01BTP9T2WCE1PAJR2DWYXG805V(2017-09-23 02:42:23.346534): Invoked with revision 3 + (2) > action metadata:01BZZKMW60DV2CJZ858QZ93HRS(2017-09-23 04:12:31.465342): Invoked with revision 4 get configdocs @@ -653,8 +675,8 @@ differences between the 'committed' and 'buffer' revision (default behavior). prior commit. If no documents have been loaded into the buffer for this collection, this will return an empty response (default) -Samples -^^^^^^^ +Sample +^^^^^^ :: @@ -877,7 +899,6 @@ is an optional parameter. Sample ^^^^^^ - :: $ shipyard logs step/01C9VVQSCFS7V9QB5GBS3WFVSE/action_xcom @@ -928,7 +949,6 @@ Provides topical help for shipyard. Sample ^^^^^^ - :: $ shipyard help diff --git a/doc/source/_static/shipyard.policy.yaml.sample b/doc/source/_static/shipyard.policy.yaml.sample index aebbbcb9..1692ac5b 100644 --- a/doc/source/_static/shipyard.policy.yaml.sample +++ b/doc/source/_static/shipyard.policy.yaml.sample @@ -62,6 +62,11 @@ # GET /api/v1.0/workflows/{id} #"workflow_orchestrator:get_workflow": "rule:admin_required" +# Retrieve the details for a note. Further authorization is required +# depending on the topic of the note +# GET /api/v1.0/notedetails/{note_id} +#"workflow_orchestrator:get_notedetails": "rule:admin_required" + # Retrieve the statuses for the site # GET /api/v1.0/site_statuses #"workflow_orchestrator:get_site_statuses": "rule:admin_required" diff --git a/src/bin/shipyard_airflow/etc/shipyard/policy.yaml.sample b/src/bin/shipyard_airflow/etc/shipyard/policy.yaml.sample index aebbbcb9..1692ac5b 100644 --- a/src/bin/shipyard_airflow/etc/shipyard/policy.yaml.sample +++ b/src/bin/shipyard_airflow/etc/shipyard/policy.yaml.sample @@ -62,6 +62,11 @@ # GET /api/v1.0/workflows/{id} #"workflow_orchestrator:get_workflow": "rule:admin_required" +# Retrieve the details for a note. Further authorization is required +# depending on the topic of the note +# GET /api/v1.0/notedetails/{note_id} +#"workflow_orchestrator:get_notedetails": "rule:admin_required" + # Retrieve the statuses for the site # GET /api/v1.0/site_statuses #"workflow_orchestrator:get_site_statuses": "rule:admin_required" diff --git a/src/bin/shipyard_airflow/shipyard_airflow/common/notes/errors.py b/src/bin/shipyard_airflow/shipyard_airflow/common/notes/errors.py index d79f2008..7040cc75 100644 --- a/src/bin/shipyard_airflow/shipyard_airflow/common/notes/errors.py +++ b/src/bin/shipyard_airflow/shipyard_airflow/common/notes/errors.py @@ -42,3 +42,28 @@ class NotesStorageError(NotesError): Raised when there is an error attempting to store a note. """ pass + + +class NoteNotFoundError(NotesRetrievalError): + """NoteNotFoundError + + Raised when a note is looked up directly and not found + """ + pass + + +class NoteURLNotSpecifiedError(NotesRetrievalError): + """NoteURLNotSpecifiedError + + Raised when a note's url info is requested for a note that has no URL + """ + pass + + +class NoteURLRetrievalError(NotesRetrievalError): + """NoteURLRetrievalError + + Raised when there is an error retrieving the URL information for a note + that has a URL specified. + """ + pass diff --git a/src/bin/shipyard_airflow/shipyard_airflow/common/notes/notes.py b/src/bin/shipyard_airflow/shipyard_airflow/common/notes/notes.py index af057d8f..d17620f1 100644 --- a/src/bin/shipyard_airflow/shipyard_airflow/common/notes/notes.py +++ b/src/bin/shipyard_airflow/shipyard_airflow/common/notes/notes.py @@ -30,6 +30,8 @@ from requests.exceptions import HTTPError from requests.exceptions import RequestException import ulid +from .errors import NoteURLNotSpecifiedError +from .errors import NoteURLRetrievalError from .errors import NotesInitializationError from .errors import NotesRetrievalError from .errors import NotesStorageError @@ -93,8 +95,9 @@ class NotesManager: immediately upon creation, if true """ n = Note(assoc_id, subject, sub_type, note_val, - verbosity=None, link_url=None, is_auth_link=None, - note_id=None, note_timestamp=None) + verbosity=verbosity, link_url=link_url, + is_auth_link=is_auth_link, note_id=note_id, + note_timestamp=note_timestamp) if store: return self.store(n) else: @@ -122,6 +125,8 @@ class NotesManager: """Retrieve a list of notes :param query: a query object to retrieve notes + :returns: a list of notes matchin the query, or [] if there are no + notes matching the query. """ try: notes = list(self.storage.retrieve(query)) @@ -132,45 +137,41 @@ class NotesManager: raise NotesRetrievalError( "Unhandled error during retrieval of notes" ) - # Get the auth token once per retrieve, not once per note. - if notes: - auth_token = self.get_token() - # resolve the note urls - # TODO: threaded? - for note in notes: - self._resolve_note_url(note, auth_token) + for note in notes: + if note.link_url: + note.resolved_url_value = ( + "Details at notedetails/{}".format(note.note_id)) return notes - def _resolve_note_url(self, note, auth_token): - """Resolve and set the value obtained from the URL for a Note. + def retrieve_by_id(self, note_id): + """Return a single note looked up by the specified note_id - :param note: the Note object to retreive and set the value for. - :param auth_token: the authorization token set as a header for the URL - request if one is indicated as needed by the note. - - If there is data retrieved at the note's url, set the - resolved_url_value with those contents. - - If there is no url for the note, return, with resolved_url_value as - None - - If there is no data retrieved, resolved_url_value for the note remains - None - - If there is an error related to retreiving the note's url value, the - resolved_url_value is set to a placeholder value indicating that the - value could not be obtained. + :param note_id: the ULID of the note to retrieve. + :raises NoteNotFoundError: if there is no note matching the requested + note_id """ + return self.storage.retrieve_by_id(note_id) + + def get_note_url_info(self, note): + """Resolve and return the value obtained from the URL for a Note. + + :param note: the note object or id of the note to retreive and set the + value for. + :returns: The contents retrieved from the note's URL. + :raises NoteNotFoundError: when the note (id) specified does not match + a known note + :raises NoteURLNotSpecifiedError: when the note has no url specified + :raises NoteURLRetrievalError: when there is an error using the + note's specified URL. + """ + # if the note is not a note, try to fetch it like an ID if not isinstance(note, Note): - LOG.debug( - "Note is None or not a Note object. URL will not be resolved" - ) - return - + note = self.retrieve_by_id(note) if not note.link_url: - LOG.debug("Note %s has no link to resolve", note.note_id) - return + LOG.debug("Note %s has no URL to resolve.", note.note_id) + raise NoteURLNotSpecifiedError() + auth_token = self.get_token() contents = None try: headers = {} @@ -186,32 +187,11 @@ class NotesManager: response.raise_for_status() # Set the valid response text to the note - note.resolved_url_value = response.text + return response.text - except HTTPError as he: - # A bad status code - don't stop, but log and indicate in note. - LOG.info( - "Note %s has a url returning a bad status code: %s", - note.note_id, response.status_code - ) - note.resolved_url_value = ( - "Note contents could not be retrieved. URL lookup failed " - "with status code: {}" - ).format(response.status_code) - except RequestException as rex: - # A more serious exception; log and indicate in the note - LOG.exception(rex) - note.resolved_url_value = ( - "Note contents could not be retrieved. URL lookup was unable " - "to complete" - ) - except Exception as ex: - # Whatever's left, log and indicate in the note - LOG.exception(ex) - note.resolved_url_value = ( - "Note contents could not be retrieved due to unexpected " - "circumstances" - ) + except (HTTPError, RequestException) as lookup_err: + LOG.exception(lookup_err) + raise NoteURLRetrievalError() class Note: @@ -316,3 +296,15 @@ class NotesStorage(metaclass=abc.ABCMeta): results. """ pass + + @abc.abstractmethod + def retrieve_by_id(self, note_id): + """Lookup a note by note_id + + :param note_id: The ID of the note to retrieve + :returns: a single Note object matching the id or None if there is no + note matching the ID. + :raises NotesRetrievalError: if there is a failure to retrieve the + note + """ + pass diff --git a/src/bin/shipyard_airflow/shipyard_airflow/common/notes/notes_helper.py b/src/bin/shipyard_airflow/shipyard_airflow/common/notes/notes_helper.py index 6361125b..b8c081a2 100644 --- a/src/bin/shipyard_airflow/shipyard_airflow/common/notes/notes_helper.py +++ b/src/bin/shipyard_airflow/shipyard_airflow/common/notes/notes_helper.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. # +from enum import Enum import logging from .notes import MAX_VERBOSITY @@ -20,41 +21,105 @@ from .notes import Query LOG = logging.getLogger(__name__) -# Constants and magic numbers for actions: -# [7:33] to slice a string like: -# -# Text: action/12345678901234567890123456 -# | | -# Position: 0....5.|..1....1....2....2....3..|.3 -# | 0 5 0 5 0 | 5 -# | | -# (7) ACTION_ID_START | -# (33) ACTION_ID_END -# -# matching the patterns in this helper. -# -ACTION_KEY_PATTERN = "action/{}" -ACTION_LOOKUP_PATTERN = "action/" -ACTION_ID_START = 7 -ACTION_ID_END = 33 -# Constants and magic numbers for steps: -# [32:] to slice a step name pattern -# step/{action_id}/{step_name} -# -# Text: step/12345678901234567890123456/my_step -# Position: 0....5....1....1....2....2....3||..3....4 -# | 0 5 0 5 0|| 5 0 -# | |\ -# (5) STEP_ACTION_ID_START | \ -# | (32) STEP_ID_START -# (31) STEP_ACTION_ID_END -# -STEP_KEY_PATTERN = "step/{}/{}" -STEP_LOOKUP_PATTERN = "step/{}" -STEP_ACTION_ID_START = 5 -STEP_ACTION_ID_END = 31 -STEP_ID_START = 32 +class _NoteType: + """Define the patterns and pertinent positions of information for a note + + :param root: the string used as the initial identifier for the type + :param key_pattern_count: the number of variable positions in the key. + E.g.: a value of 3, and a root of "tacos" would generate a key_pattern + of "tacos/{}/{}/{}", while a value of 0 would generate "tacos". + The lookup_pattern for a type is also drived from the key_pattern_count + by subtracting 1 (minimum 0), such that a value of 3 and a root of + "tacos" would generate a lookup_pattern of "tacos/{}/{}/ + :param id_start: the start location in the assoc_id of a note of this type + where the id of the item it is associated with appears. + :param id_end: the optional end location in the assoc_id for locating the + id from the assoc_id of a note of this type. This is only valid for + items that have a fixed length key. + :param default_subtype: the default sub_type specified upon creation of a + note of this type. + """ + + def __init__(self, root, key_pattern_count, id_start, + id_end=None, default_subtype="metadata"): + self.root = root + self.base_pattern = "{}/".format(root) + self.kp_count = key_pattern_count + self.key_pattern = "{}{}".format(root, "/{}" * self.kp_count) + self.lp_count = self.kp_count - 1 if self.kp_count > 0 else 0 + self.lookup_pattern = "{}/{}".format(root, "{}/" * self.lp_count) + self.id_start = id_start + self.id_end = id_end + self.default_subtype = default_subtype + + +class NoteType(Enum): + + # Action + # + # Text: action/12345678901234567890123456 + # | | + # Position: 0....5.|..1....1....2....2....3..|.3 + # | 0 5 0 5 0 | 5 + # | | + # (7) ACTION_ID_START | + # (33) ACTION_ID_END + ACTION = _NoteType( + root="action", + key_pattern_count=1, + id_start=7, + id_end=33, + default_subtype="action metadata") + + # Step + # + # Text: step/12345678901234567890123456/my_step + # | || + # Position: 0....5....1....1....2....2....3||..3....4 + # | 0 5 0 5 0|| 5 0 + # | |\ + # (5) STEP_ACTION_ID_START | \ + # | (32) STEP_ID_START + # (31) STEP_ACTION_ID_END + STEP = _NoteType( + root="step", + key_pattern_count=2, + id_start=32, + default_subtype="step metadata") + + OTHER = _NoteType(root="", key_pattern_count=0, id_start=0) + + @property + def base_pattern(self): + return self.value.base_pattern + + @property + def key_pattern(self): + return self.value.key_pattern + + @property + def lookup_pattern(self): + return self.value.lookup_pattern + + @property + def id_start(self): + return self.value.id_start + + @property + def id_end(self): + return self.value.id_end + + @property + def default_subtype(self): + return self.value.default_subtype + + @classmethod + def get_type(cls, note): + for tp in cls: + if note.assoc_id.startswith(tp.value.base_pattern): + return tp + return cls.OTHER class NotesHelper: @@ -68,7 +133,7 @@ class NotesHelper: def _failsafe_make_note(self, assoc_id, subject, sub_type, note_val, verbosity=MIN_VERBOSITY, link_url=None, - is_auth_link=None): + is_auth_link=None, note_timestamp=None): """LOG and continue on any note creation failure""" try: self.nm.create( @@ -79,6 +144,7 @@ class NotesHelper: verbosity=verbosity, link_url=link_url, is_auth_link=is_auth_link, + note_timestamp=note_timestamp ) except Exception as ex: LOG.warn( @@ -105,13 +171,45 @@ class NotesHelper: LOG.exception(ex) return [] + # + # Retrieve notes by note ID + # + + def get_note(self, note_id): + """Return a single note looked up by the specified note_id + + :param note_id: the ULID of the note to retrieve. + :raises NoteNotFoundError: if there is no note matching the requested + note_id + """ + return self.nm.retrieve_by_id(note_id) + + def get_note_details(self, note): + """Return the note details for the specified note + + :param note: the Note object with additional details to retrieve. + """ + return self.nm.get_note_url_info(note) + + def get_note_assoc_id_type(self, note): + """Return the type of note based on the assoc_id + + :param note: The note to examine + + The purpose of this method is to use the standard formats (e.g.: + action and step) supported by this helper to get the type of note. + This value can be used by a client to enforce access rules to the note + based on the item it is related to. + """ + return NoteType.get_type(note) + # # Action notes helper methods # def make_action_note(self, action_id, note_val, subject=None, sub_type=None, verbosity=MIN_VERBOSITY, link_url=None, - is_auth_link=None): + is_auth_link=None, note_timestamp=None): """Creates an action note using a convention for the note's assoc_id :param action_id: the ULID id of an action @@ -127,12 +225,14 @@ class NotesHelper: :param is_auth_link: optional, defaults to False, indicating if there is a need to send a Shipyard service account token with the request to the optional URL + :param note_timestamp: the parseable string timestamp to associate with + this note. Optional, defaults to the creation time of the note. """ - assoc_id = ACTION_KEY_PATTERN.format(action_id) + assoc_id = NoteType.ACTION.key_pattern.format(action_id) if subject is None: subject = action_id if sub_type is None: - sub_type = "action metadata" + sub_type = NoteType.ACTION.default_subtype self._failsafe_make_note( assoc_id=assoc_id, @@ -142,6 +242,7 @@ class NotesHelper: verbosity=verbosity, link_url=link_url, is_auth_link=is_auth_link, + note_timestamp=note_timestamp ) def get_all_action_notes(self, verbosity=MIN_VERBOSITY): @@ -150,19 +251,17 @@ class NotesHelper: :param verbosity: optional integer, 0-5, the maximum verbosity level to retrieve, defaults to 1 (most summary level) if set to less than 1, returns {}, skipping any retrieval - - Warning: if there are a lot of URL links in notes, this could take a - long time. The default verbosity of 1 attempts to avoid this as there - is less expectation of URL links on summary notes. """ notes = self._failsafe_get_notes( - assoc_id_pattern=ACTION_LOOKUP_PATTERN, + assoc_id_pattern=NoteType.ACTION.lookup_pattern, verbosity=verbosity, exact_match=False ) note_dict = {} + id_s = NoteType.ACTION.id_start + id_e = NoteType.ACTION.id_end for n in notes: - action_id = n.assoc_id[ACTION_ID_START:ACTION_ID_END] + action_id = n.assoc_id[id_s:id_e] if action_id not in note_dict: note_dict[action_id] = [] note_dict[action_id].append(n) @@ -178,7 +277,7 @@ class NotesHelper: """ return self._failsafe_get_notes( - assoc_id_pattern=ACTION_KEY_PATTERN.format(action_id), + assoc_id_pattern=NoteType.ACTION.key_pattern.format(action_id), verbosity=verbosity, exact_match=True ) @@ -189,7 +288,7 @@ class NotesHelper: def make_step_note(self, action_id, step_id, note_val, subject=None, sub_type=None, verbosity=MIN_VERBOSITY, link_url=None, - is_auth_link=None): + is_auth_link=None, note_timestamp=None): """Creates an action note using a convention for the note's assoc_id :param action_id: the ULID id of the action containing the note @@ -206,12 +305,14 @@ class NotesHelper: :param is_auth_link: optional, defaults to False, indicating if there is a need to send a Shipyard service account token with the request to the optional URL + :param note_timestamp: the parseable string timestamp to associate with + this note. Optional, defaults to the creation time of the note. """ - assoc_id = STEP_KEY_PATTERN.format(action_id, step_id) + assoc_id = NoteType.STEP.key_pattern.format(action_id, step_id) if subject is None: subject = step_id if sub_type is None: - sub_type = "step metadata" + sub_type = NoteType.STEP.default_subtype self._failsafe_make_note( assoc_id=assoc_id, @@ -221,6 +322,7 @@ class NotesHelper: verbosity=verbosity, link_url=link_url, is_auth_link=is_auth_link, + note_timestamp=note_timestamp ) def get_all_step_notes_for_action(self, action_id, @@ -233,13 +335,14 @@ class NotesHelper: if set to less than 1, returns {}, skipping any retrieval """ notes = self._failsafe_get_notes( - assoc_id_pattern=STEP_LOOKUP_PATTERN.format(action_id), + assoc_id_pattern=NoteType.STEP.lookup_pattern.format(action_id), verbosity=verbosity, exact_match=False ) note_dict = {} + id_s = NoteType.STEP.id_start for n in notes: - step_id = n.assoc_id[STEP_ID_START:] + step_id = n.assoc_id[id_s:] if step_id not in note_dict: note_dict[step_id] = [] note_dict[step_id].append(n) @@ -256,7 +359,8 @@ class NotesHelper: """ return self._failsafe_get_notes( - assoc_id_pattern=STEP_KEY_PATTERN.format(action_id, step_id), + assoc_id_pattern=NoteType.STEP.key_pattern.format( + action_id, step_id), verbosity=verbosity, exact_match=True ) diff --git a/src/bin/shipyard_airflow/shipyard_airflow/common/notes/storage_impl_db.py b/src/bin/shipyard_airflow/shipyard_airflow/common/notes/storage_impl_db.py index ead5a70f..9e96fc94 100644 --- a/src/bin/shipyard_airflow/shipyard_airflow/common/notes/storage_impl_db.py +++ b/src/bin/shipyard_airflow/shipyard_airflow/common/notes/storage_impl_db.py @@ -34,6 +34,7 @@ from sqlalchemy.orm import sessionmaker from .notes import Note from .notes import NotesStorage +from .errors import NoteNotFoundError from .errors import NotesError from .errors import NotesInitializationError @@ -128,19 +129,27 @@ class ShipyardSQLNotesStorage(NotesStorage): TNote.assoc_id == a_id_pat, TNote.verbosity <= max_verb ) - ) + ).order_by(TNote.note_timestamp) else: n_qry = session.query(TNote).filter( and_( TNote.assoc_id.like(a_id_pat + '%'), TNote.verbosity <= max_verb ) - ) + ).order_by(TNote.note_timestamp) db_notes = n_qry.all() for tn in db_notes: r_notes.append(self._map(tn, Note)) return r_notes + def retrieve_by_id(self, note_id): + with self.session_scope() as session: + note = session.query(TNote).filter( + TNote.note_id == note_id).one_or_none() + if not note: + raise NoteNotFoundError() + return self._map(note, Note) + def _map(self, src, target_type): """Maps a Note object to/from a TNote object. diff --git a/src/bin/shipyard_airflow/shipyard_airflow/common/notes/storage_impl_mem.py b/src/bin/shipyard_airflow/shipyard_airflow/common/notes/storage_impl_mem.py index 76b3aab6..2c83f846 100644 --- a/src/bin/shipyard_airflow/shipyard_airflow/common/notes/storage_impl_mem.py +++ b/src/bin/shipyard_airflow/shipyard_airflow/common/notes/storage_impl_mem.py @@ -13,6 +13,7 @@ # limitations under the License. # """Implementations of the NotesStorage base class""" +from .errors import NoteNotFoundError from .notes import NotesStorage @@ -41,4 +42,11 @@ class MemoryNotesStorage(NotesStorage): if (note.assoc_id.startswith(pat) and note.verbosity <= max_verb): notes.append(note) + notes.sort(key=lambda x: x.note_timestamp) return notes + + def retrieve_by_id(self, note_id): + note = self.storage.get(note_id) + if not note: + raise NoteNotFoundError() + return note diff --git a/src/bin/shipyard_airflow/shipyard_airflow/control/api.py b/src/bin/shipyard_airflow/shipyard_airflow/control/api.py index 22b0893b..c92af916 100644 --- a/src/bin/shipyard_airflow/shipyard_airflow/control/api.py +++ b/src/bin/shipyard_airflow/shipyard_airflow/control/api.py @@ -42,6 +42,7 @@ from shipyard_airflow.control.middleware.common_params import \ CommonParametersMiddleware from shipyard_airflow.control.middleware.context import ContextMiddleware from shipyard_airflow.control.middleware.logging_mw import LoggingMiddleware +from shipyard_airflow.control.notes.notedetails_api import NoteDetailsResource from shipyard_airflow.control.status.status_api import StatusResource from shipyard_airflow.errors import (AppError, default_error_serializer, default_exception_handler) @@ -78,6 +79,7 @@ def start_api(): ('/configdocs', ConfigDocsStatusResource()), ('/configdocs/{collection_id}', ConfigDocsResource()), ('/commitconfigdocs', CommitConfigDocsResource()), + ('/notedetails/{note_id}', NoteDetailsResource()), ('/renderedconfigdocs', RenderedConfigDocsResource()), ('/workflows', WorkflowResource()), ('/workflows/{workflow_id}', WorkflowIdResource()), diff --git a/src/bin/shipyard_airflow/shipyard_airflow/control/notes/__init__.py b/src/bin/shipyard_airflow/shipyard_airflow/control/notes/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/src/bin/shipyard_airflow/shipyard_airflow/control/notes/notedetails_api.py b/src/bin/shipyard_airflow/shipyard_airflow/control/notes/notedetails_api.py new file mode 100644 index 00000000..ece1ce80 --- /dev/null +++ b/src/bin/shipyard_airflow/shipyard_airflow/control/notes/notedetails_api.py @@ -0,0 +1,105 @@ +# Copyright 2018 AT&T Intellectual Property. All other 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. +import falcon + +from shipyard_airflow.common.notes.errors import NoteNotFoundError +from shipyard_airflow.common.notes.errors import NoteURLNotSpecifiedError +from shipyard_airflow.common.notes.errors import NoteURLRetrievalError +from shipyard_airflow.common.notes.notes_helper import NoteType +from shipyard_airflow.control.base import BaseResource +from shipyard_airflow.control.helpers.notes import NOTES as notes_helper +from shipyard_airflow.errors import ApiError +from shipyard_airflow.errors import InvalidFormatError +from shipyard_airflow import policy + + +NOTE_TYPE_RBAC = { + NoteType.ACTION: policy.GET_ACTION, + NoteType.STEP: policy.GET_ACTION_STEP, + # Anything else uses only the already checked GET_NOTEDETAILS + # new known note types should be added to the notes helper and also + # represented here. + NoteType.OTHER: None +} + + +# /api/v1.0/notedetails/{note_id} +class NoteDetailsResource(BaseResource): + """Resource to service requests for note details""" + @policy.ApiEnforcer(policy.GET_NOTEDETAILS) + def on_get(self, req, resp, **kwargs): + """Retrieves additional information for a note. + + Using the specified note_id, looks up any additional information + for a note + """ + note_id = kwargs['note_id'] + self.validate_note_id(note_id) + note = self.get_note_with_access_check(req.context, note_id) + resp.body = self.get_note_details(note) + resp.status = falcon.HTTP_200 + + def validate_note_id(self, note_id): + if not len(note_id) == 26: + raise InvalidFormatError( + title="Notes ID values are 26 character ULID values", + description="Invalid note_id: {} in URL".format(note_id) + ) + + def get_note_with_access_check(self, context, note_id): + """Retrieve the note and checks user access to the note + + :param context: the request context + :param note_id: the id of the note to retrieve. + :returns: the note + """ + try: + note = notes_helper.get_note(note_id) + note_type = notes_helper.get_note_assoc_id_type(note) + if note_type not in NOTE_TYPE_RBAC: + raise ApiError( + title="Unable to check permission for note type", + description=( + "Shipyard is not correctly identifying note type " + "for note {}".format(note_id)), + status=falcon.HTTP_500, + retry=False) + policy.check_auth(context, NOTE_TYPE_RBAC[note_type]) + return note + except NoteNotFoundError: + raise ApiError( + title="No note found", + description=("Note {} is not found".format(note_id)), + status=falcon.HTTP_404) + + def get_note_details(self, note): + """Retrieve the note details from the notes_helper + + :param note: the note with extended information + """ + try: + return notes_helper.get_note_details(note) + except NoteURLNotSpecifiedError: + raise ApiError( + title="No further note details are available", + description=("Note {} has no additional information to " + "return".format(note.note_id)), + status=falcon.HTTP_404) + except NoteURLRetrievalError: + raise ApiError( + title="Unable to retrieve URL information for note", + description=("Note {} has additional information, but it " + "cannot be accessed by Shipyard at this " + "time".format(note.note_id)), + status=falcon.HTTP_500) diff --git a/src/bin/shipyard_airflow/shipyard_airflow/policy.py b/src/bin/shipyard_airflow/shipyard_airflow/policy.py index 5e10aae8..731503c2 100644 --- a/src/bin/shipyard_airflow/shipyard_airflow/policy.py +++ b/src/bin/shipyard_airflow/shipyard_airflow/policy.py @@ -40,6 +40,7 @@ COMMIT_CONFIGDOCS = 'workflow_orchestrator:commit_configdocs' GET_RENDEREDCONFIGDOCS = 'workflow_orchestrator:get_renderedconfigdocs' LIST_WORKFLOWS = 'workflow_orchestrator:list_workflows' GET_WORKFLOW = 'workflow_orchestrator:get_workflow' +GET_NOTEDETAILS = 'workflow_orchestrator:get_notedetails' GET_SITE_STATUSES = 'workflow_orchestrator:get_site_statuses' ACTION_DEPLOY_SITE = 'workflow_orchestrator:action_deploy_site' ACTION_UPDATE_SITE = 'workflow_orchestrator:action_update_site' @@ -206,6 +207,16 @@ class ShipyardPolicy(object): 'method': 'GET' }] ), + policy.DocumentedRuleDefault( + GET_NOTEDETAILS, + RULE_ADMIN_REQUIRED, + ('Retrieve the details for a note. Further authorization is ' + 'required depending on the topic of the note'), + [{ + 'path': '/api/v1.0/notedetails/{note_id}', + 'method': 'GET' + }] + ), policy.DocumentedRuleDefault( GET_SITE_STATUSES, RULE_ADMIN_REQUIRED, diff --git a/src/bin/shipyard_airflow/tests/unit/common/notes/test_notes.py b/src/bin/shipyard_airflow/tests/unit/common/notes/test_notes.py index 397bd117..1a70656b 100644 --- a/src/bin/shipyard_airflow/tests/unit/common/notes/test_notes.py +++ b/src/bin/shipyard_airflow/tests/unit/common/notes/test_notes.py @@ -20,7 +20,10 @@ import responses from shipyard_airflow.common.notes.errors import ( NotesInitializationError, NotesRetrievalError, - NotesStorageError + NotesStorageError, + NoteNotFoundError, + NoteURLNotSpecifiedError, + NoteURLRetrievalError ) from shipyard_airflow.common.notes.notes import ( Note, @@ -42,6 +45,9 @@ class NotesStorageErrorImpl(NotesStorage): def retrieve(self, query): raise Exception("Outta Nowhere") + def retrieve_by_id(self, note_id): + raise Exception("No warning") + class NotesStorageExpectedErrorImpl(NotesStorage): def store(self, note): @@ -50,6 +56,9 @@ class NotesStorageExpectedErrorImpl(NotesStorage): def retrieve(self, query): raise NotesRetrievalError("Expected") + def retrieve_by_id(self, note_id): + raise NotesRetrievalError("Expected") + class TestNotesManager: def test_init(self): @@ -191,23 +200,8 @@ class TestNotesManager: n_list = nm.retrieve(Query("test1/11111/aaa", exact_match=True)) assert len(n_list) == 1 - @responses.activate - def test_store_retrieve_urls(self): - responses.add( - method="GET", - url="http://test.test", - body="Hello from testland", - status=200, - content_type="text/plain" - ) - responses.add( - method="GET", - url="http://test.test2", - body="Hello from testland2", - status=200, - content_type="text/plain" - ) - + def test_store_retrieve_url_refs(self): + """Tests that notes retrieved as a list have notedetails refs""" nm = NotesManager(MemoryNotesStorage(), get_token) nm.store(Note( assoc_id="test1/11111/aaa", @@ -226,7 +220,46 @@ class TestNotesManager: n_list = nm.retrieve(Query("test1")) assert len(n_list) == 2 for n in n_list: - assert n.resolved_url_value.startswith("Hello from testland") + assert n.resolved_url_value == ( + "Details at notedetails/" + n.note_id) + + @responses.activate + def test_store_retrieve_urls(self): + responses.add( + method="GET", + url="http://test.test", + body="Hello from testland", + status=200, + content_type="text/plain" + ) + responses.add( + method="GET", + url="http://test.test2", + body="Hello from testland2", + status=200, + content_type="text/plain" + ) + nm = NotesManager(MemoryNotesStorage(), get_token) + nm.store(Note( + assoc_id="test1/11111/aaa", + subject="store_retrieve3", + sub_type="test", + note_val="this is my note 1", + link_url="http://test.test/" + )) + nm.store(Note( + assoc_id="test1/11111/bbb", + subject="store_retrieve3", + sub_type="test", + note_val="this is my note 2", + link_url="http://test.test2/" + )) + n_list = nm.retrieve(Query("test1")) + assert len(n_list) == 2 + for n in n_list: + assert n.resolved_url_value.startswith("Details at notedetails/") + assert nm.get_note_url_info(n.note_id).startswith( + "Hello from testland") with pytest.raises(KeyError): auth_hdr = responses.calls[0].request.headers['X-Auth-Token'] @@ -265,10 +298,14 @@ class TestNotesManager: n_list = nm.retrieve(Query("test1")) assert len(n_list) == 2 for n in n_list: + assert n.resolved_url_value == ( + "Details at notedetails/" + n.note_id) if n.assoc_id == "test1/11111/aaa": - assert "failed with status code: 404" in n.resolved_url_value + with pytest.raises(NoteURLRetrievalError): + nd = nm.get_note_url_info(n.note_id) else: - assert n.resolved_url_value.startswith("Hello from testland") + nd = nm.get_note_url_info(n.note_id) + assert nd.startswith("Hello from testland") @responses.activate def test_store_retrieve_url_does_not_exist(self): @@ -299,9 +336,33 @@ class TestNotesManager: assert len(n_list) == 2 for n in n_list: if n.assoc_id == "test1/11111/aaa": - assert "URL lookup was unable" in n.resolved_url_value + with pytest.raises(NoteURLRetrievalError): + nd = nm.get_note_url_info(n.note_id) else: - assert n.resolved_url_value.startswith("Hello from testland") + nd = nm.get_note_url_info(n.note_id) + assert nd.startswith("Hello from testland") + + def test_store_retrieve_url_not_specified(self): + nm = NotesManager(MemoryNotesStorage(), get_token) + nm.store(Note( + assoc_id="test1/11111/aaa", + subject="store_retrieve3", + sub_type="test", + note_val="this is my note 1", + link_url="" + )) + nm.store(Note( + assoc_id="test1/11111/bbb", + subject="store_retrieve3", + sub_type="test", + note_val="this is my note 2", + link_url=None + )) + n_list = nm.retrieve(Query("test1")) + assert len(n_list) == 2 + for n in n_list: + with pytest.raises(NoteURLNotSpecifiedError): + nd = nm.get_note_url_info(n.note_id) @responses.activate def test_store_retrieve_with_auth(self): @@ -325,7 +386,8 @@ class TestNotesManager: n_list = nm.retrieve(Query("test1")) assert len(n_list) == 1 for n in n_list: - assert n.resolved_url_value == "Hello from testland2" + nd = nm.get_note_url_info(n.note_id) + assert nd == "Hello from testland2" auth_hdr = responses.calls[0].request.headers['X-Auth-Token'] assert 'token' == auth_hdr diff --git a/src/bin/shipyard_client/shipyard_client/api_client/shipyard_api_client.py b/src/bin/shipyard_client/shipyard_client/api_client/shipyard_api_client.py index cf1590c5..d91b06c6 100644 --- a/src/bin/shipyard_client/shipyard_client/api_client/shipyard_api_client.py +++ b/src/bin/shipyard_client/shipyard_client/api_client/shipyard_api_client.py @@ -34,6 +34,7 @@ class ApiPaths(enum.Enum): GET_STEP_DETAIL = _BASE_URL + 'actions/{}/steps/{}' GET_STEP_LOG = _BASE_URL + 'actions/{}/steps/{}/logs' POST_CONTROL_ACTION = _BASE_URL + 'actions/{}/control/{}' + GET_NOTEDETAILS = _BASE_URL + 'notedetails/{}' GET_WORKFLOWS = _BASE_URL + 'workflows' GET_DAG_DETAIL = _BASE_URL + 'workflows/{}' GET_SITE_STATUSES = _BASE_URL + 'site_statuses' @@ -225,6 +226,15 @@ class ShipyardClient(BaseClient): self.get_endpoint(), action_id, control_verb) return self.post_resp(url) + def get_note_details(self, note_id): + """Retrieve note details from a note's associated information + + :param note_id: The ID of the note having additional information + """ + url = ApiPaths.GET_NOTEDETAILS.value.format( + self.get_endpoint(), note_id) + return self.get_resp(url) + def get_workflows(self, since=None): """ Queries airflow for DAGs that are running or have run diff --git a/src/bin/shipyard_client/shipyard_client/cli/cli_format_common.py b/src/bin/shipyard_client/shipyard_client/cli/cli_format_common.py index 983edbf5..bc5ee9b4 100644 --- a/src/bin/shipyard_client/shipyard_client/cli/cli_format_common.py +++ b/src/bin/shipyard_client/shipyard_client/cli/cli_format_common.py @@ -25,7 +25,7 @@ def gen_action_steps(step_list, action_id): """ # Generate the steps table. steps = format_utils.table_factory( - field_names=['Steps', 'Index', 'State', 'Notes'] + field_names=['Steps', 'Index', 'State', 'Footnotes'] ) # rendered notes , a list of lists of notes r_notes = [] @@ -34,7 +34,7 @@ def gen_action_steps(step_list, action_id): for step in step_list: notes = step.get('notes') if notes: - r_notes.append(format_utils.format_notes(notes)) + r_notes.append(format_notes(notes)) steps.add_row([ 'step/{}/{}'.format(action_id, step.get('id')), step.get('index'), @@ -44,16 +44,9 @@ def gen_action_steps(step_list, action_id): else: steps.add_row(['None', '', '', '']) - table_string = format_utils.table_get_string(steps) - - if r_notes: - note_index = 1 - for note_list in r_notes: - table_string += "\n\n({}):\n\n{}".format( - note_index, "\n".join(note_list) - ) - note_index += 1 - return table_string + return "{}\n\n{}".format( + format_utils.table_get_string(steps), + notes_table("Step", r_notes)) def gen_action_commands(command_list): @@ -141,7 +134,7 @@ def gen_action_table(action_list): """ actions = format_utils.table_factory( field_names=['Name', 'Action', 'Lifecycle', 'Execution Time', - 'Step Succ/Fail/Oth', 'Notes']) + 'Step Succ/Fail/Oth', 'Footnotes']) # list of lists of rendered notes r_notes = [] if action_list: @@ -149,7 +142,7 @@ def gen_action_table(action_list): for action in sorted(action_list, key=lambda k: k['id']): notes = action.get('notes') if notes: - r_notes.append(format_utils.format_notes(notes)) + r_notes.append(format_notes(notes)) actions.add_row([ action.get('name'), 'action/{}'.format(action.get('id')), @@ -161,16 +154,9 @@ def gen_action_table(action_list): else: actions.add_row(['None', '', '', '', '', '']) - table_string = format_utils.table_get_string(actions) - - if r_notes: - note_index = 1 - for note_list in r_notes: - table_string += "\n\n({}):\n\n{}".format( - note_index, "\n".join(note_list) - ) - note_index += 1 - return table_string + return "{}\n\n{}".format( + format_utils.table_get_string(actions), + notes_table("Action", r_notes)) def _step_summary(step_list): @@ -370,13 +356,88 @@ def _site_statuses_switcher(status_type): return call_func -def gen_detail_notes(dict_with_notes): + +def gen_detail_notes(title, dict_with_notes): """Generates a standard formatted section of notes + :param title: the title for the notes section. E.g.: "Step" :param dict_with_notes: a dictionary with a possible notes field. :returns: string of notes or empty string if there were no notes """ - n_strings = format_utils.format_notes(dict_with_notes.get('notes', [])) + n_strings = format_notes(dict_with_notes.get('notes', [])) if n_strings: - return "Notes:\n{}".format("\n".join(n_strings)) + return "{} Notes:\n{}".format(title, "\n".join(n_strings)) return "" + + +def notes_table(title, notes_list): + """Format a table of notes + + :param title: the header for the table. e.g.: "Step" + :param list notes_list: a list of lists of formatted notes: + e.g.:[[note1,note2],[note3]] + The notes ideally have been pre-formatted by "format_notes" + + :returns: string of a table e.g.: + + Step Notes Note + (1) > note1 + - Info avail... + > note2 + (2) > note3 + + If notes_list is empty, returns an empty string. + """ + if not notes_list: + return "" + headers = ["{} Footnotes".format(title), "Note"] + rows = [] + index = 1 + for notes in notes_list: + rows.append(["({})".format(index), "\n".join(notes)]) + index += 1 + return format_utils.table_get_string( + format_utils.table_factory(headers, rows)) + + +def format_notes(notes): + """Formats a list of notes. + + :param list notes: The list of note dictionaries to display + :returns: a list of note strings + + Assumed note dictionary example: + { + 'assoc_id': "action/12345678901234567890123456, + 'subject': "12345678901234567890123456", + 'sub_type': "action", + 'note_val': "message", + 'verbosity': 1, + 'note_id': "09876543210987654321098765", + 'note_timestamp': "2018-10-08 14:23:53.346534", + 'resolved_url_value': \ + "Details at notedetails/09876543210987654321098765" + } + + Resulting in: + + > action:12345678901234567890123456(2018-10-08 14:23:53.346534): message + - Info available with 'describe notedetails/09876543210987654321098765' + """ + nl = [] + for n in notes: + try: + s = "> {}:{}({}): {}".format( + n['sub_type'], + n['subject'], + n['note_timestamp'], + n['note_val'] + ) + if n['resolved_url_value']: + s += ("\n - Info available with " + "'describe notedetails/{}'".format(n['note_id'])) + except KeyError: + s = "!!! Unparseable Note: {}".format(n) + + nl.append(s) + return nl diff --git a/src/bin/shipyard_client/shipyard_client/cli/describe/actions.py b/src/bin/shipyard_client/shipyard_client/cli/describe/actions.py index 44bc4eae..0f8a3f5a 100644 --- a/src/bin/shipyard_client/shipyard_client/cli/describe/actions.py +++ b/src/bin/shipyard_client/shipyard_client/cli/describe/actions.py @@ -53,10 +53,48 @@ class DescribeAction(CliAction): cli_format_common.gen_action_validations( resp_j.get('validations') ), - cli_format_common.gen_detail_notes(resp_j) + cli_format_common.gen_detail_notes("Action", resp_j) ) +class DescribeNotedetails(CliAction): + """Action to Describe notedetails""" + + def __init__(self, ctx, note_id): + """Sets parameters.""" + super().__init__(ctx) + self.logger.debug( + "DescribeNotedetails action initialized with note_id=%s", note_id) + self.note_id = note_id + + def invoke(self): + """Calls API Client and formats response from API Client""" + self.logger.debug("Calling API Client get_note_details.") + return self.get_api_client().get_note_details( + note_id=self.note_id) + + # Handle 404 with default error handler for cli. + cli_handled_err_resp_codes = [400, 404, 500] + + # Handle 200 responses using the cli_format_response_handler + cli_handled_succ_resp_codes = [200] + + def cli_format_response_handler(self, response): + """CLI output handler + + :param response: a requests response object containing the details + :returns: a string representing a formatted response + + Handles 200 responses. If the response contains '\n' characters + (literally), this will attempt to replace with newline characters + """ + resp = response.text + if "\\n" in resp: + return "\n".join(resp.split("\\n")) + else: + return resp + + class DescribeStep(CliAction): """Action to Describe Step""" @@ -91,7 +129,7 @@ class DescribeStep(CliAction): resp_j = response.json() return "{}\n\n{}\n".format( cli_format_common.gen_action_step_details(resp_j, self.action_id), - cli_format_common.gen_detail_notes(resp_j) + cli_format_common.gen_detail_notes("Step", resp_j) ) diff --git a/src/bin/shipyard_client/shipyard_client/cli/describe/commands.py b/src/bin/shipyard_client/shipyard_client/cli/describe/commands.py index d4d8864a..9da41ef5 100644 --- a/src/bin/shipyard_client/shipyard_client/cli/describe/commands.py +++ b/src/bin/shipyard_client/shipyard_client/cli/describe/commands.py @@ -19,6 +19,7 @@ import click from click_default_group import DefaultGroup from shipyard_client.cli.describe.actions import DescribeAction +from shipyard_client.cli.describe.actions import DescribeNotedetails from shipyard_client.cli.describe.actions import DescribeStep from shipyard_client.cli.describe.actions import DescribeValidation from shipyard_client.cli.describe.actions import DescribeWorkflow @@ -29,7 +30,7 @@ from shipyard_client.cli.input_checks import check_id, check_workflow_id @click.pass_context def describe(ctx): """ - Describe the action, step, or validation. \n + Describe the action, step, note details or validation. \n For more information on describe commands please enter the describe command followed by '--help' \n Example: shipyard describe action --help \n @@ -41,7 +42,8 @@ def describe(ctx): FORMAT: shipyard describe \n EXAMPLE: shipyard describe action/01BTG32JW87G0YKA1K29TKNAFX | shipyard describe step/01BTG32JW87G0YKA1K29TKNAFX/preflight | shipyard describe - validation/01BTG32JW87G0YKA1K29TKNAFX/01BTG3PKBS15KCKFZ56XXXBGF2 + validation/01BTG32JW87G0YKA1K29TKNAFX/01BTG3PKBS15KCKFZ56XXXBGF2 | shipyard + describe notedetails/01BTG32JW87G0YKA1KA9AKAAB3 """ @@ -64,8 +66,11 @@ def describe_default_command(ctx, namespace_item): elif namespace[0] == 'workflow': ctx.invoke( describe_workflow, - workflow_id=namespace[1] - ) + workflow_id=namespace[1]) + elif namespace[0] == 'notedetails': + ctx.invoke( + describe_notedetails, + note_id=namespace[1]) else: raise Exception('Invalid namespaced describe action') except Exception: @@ -74,7 +79,8 @@ def describe_default_command(ctx, namespace_item): "action: action/action id\n" "step: step/action id/step id\n" "validation: validation/validation id/action id\n" - "workflow: workflow/workflow id") + "workflow: workflow/workflow id\n" + "notedetails: notedetails/note_id") DESC_ACTION = """ @@ -102,6 +108,32 @@ def describe_action(ctx, action_id): click.echo(DescribeAction(ctx, action_id).invoke_and_return_resp()) +DESC_NOTEDETAILS = """ +COMMAND: describe notedetials \n +DESCRIPTION: Retrieves the detailed information that is associated with the +specified note id. \n +FORMAT: shipyard describe notedetails \n +EXAMPLE: shipyard describe notedetails 01BTG32JW87G0YKA1KA9AKAAB3 +""" + +SHORT_DESC_NOTEDETAILS = ( + "Retrieves the detailed information about the supplied action id.") + + +@describe.command('notedetails', + help=DESC_NOTEDETAILS, short_help=SHORT_DESC_NOTEDETAILS) +@click.argument('note_id') +@click.pass_context +def describe_notedetails(ctx, note_id): + + if not note_id: + ctx.fail("A note id argument must be passed.") + + check_id(ctx, note_id) + + click.echo(DescribeNotedetails(ctx, note_id).invoke_and_return_resp()) + + DESC_STEP = """ COMMAND: describe step \n DESCRIPTION: Retrieves the step details associated with an action and step. \n diff --git a/src/bin/shipyard_client/shipyard_client/cli/format_utils.py b/src/bin/shipyard_client/shipyard_client/cli/format_utils.py index 2f6cc6e8..5ab1c9c6 100644 --- a/src/bin/shipyard_client/shipyard_client/cli/format_utils.py +++ b/src/bin/shipyard_client/shipyard_client/cli/format_utils.py @@ -234,41 +234,3 @@ def table_get_string(table, title='', vertical_char='|', align='l'): # vertical_char - Single character string used to draw vertical # lines. Default is '|'. return table.get_string(title=title, vertical_char=vertical_char) - - -def format_notes(notes): - """Formats a list of notes. - - :param list notes: The list of note dictionaries to display - :returns: a list of note strings - - Assumed note dictionary example: - { - 'assoc_id': "action/12345678901234567890123456, - 'subject': "12345678901234567890123456", - 'sub_type': "action", - 'note_val': "This is the message", - 'verbosity': 1, - 'note_id': "09876543210987654321098765", - 'note_timestamp': "2018-10-08 14:23:53.346534", - 'resolved_url_value': "
some info
- } - """ - nl = [] - for n in notes: - try: - s = "{}:{}({}): {}".format( - n['sub_type'], - n['subject'], - n['note_timestamp'], - n['note_val'] - ) - if n['resolved_url_value']: - s += "\n >>> {}".format( - n['resolved_url_value'] - ) - except KeyError: - s = "!!! Unparseable Note: {}".format(n) - - nl.append(s) - return nl diff --git a/src/bin/shipyard_client/shipyard_client/cli/input_checks.py b/src/bin/shipyard_client/shipyard_client/cli/input_checks.py index 61f254d3..acef121e 100644 --- a/src/bin/shipyard_client/shipyard_client/cli/input_checks.py +++ b/src/bin/shipyard_client/shipyard_client/cli/input_checks.py @@ -31,13 +31,13 @@ def check_control_action(ctx, action): ctx.fail('Invalid action. Please enter pause, unpause, or stop.') -def check_id(ctx, action_id): +def check_id(ctx, ulid_id): """Verifies a ULID id is in a valid format""" - if action_id is None: + if ulid_id is None: ctx.fail('Invalid ID. None is not a valid action ID.') - if len(action_id) != 26: + if len(ulid_id) != 26: ctx.fail('Invalid ID. ID can only be 26 characters.') - if not action_id.isalnum(): + if not ulid_id.isalnum(): ctx.fail('Invalid ID. ID can only contain letters and numbers.') diff --git a/src/bin/shipyard_client/tests/unit/apiclient_test/test_shipyard_api_client.py b/src/bin/shipyard_client/tests/unit/apiclient_test/test_shipyard_api_client.py index 3104c403..3198a4cd 100644 --- a/src/bin/shipyard_client/tests/unit/apiclient_test/test_shipyard_api_client.py +++ b/src/bin/shipyard_client/tests/unit/apiclient_test/test_shipyard_api_client.py @@ -196,6 +196,17 @@ def test_post_control(*args): shipyard_client.get_endpoint(), action_id, control_verb) +@mock.patch.object(BaseClient, 'post_resp', replace_post_rep) +@mock.patch.object(BaseClient, 'get_resp', replace_get_resp) +@mock.patch.object(BaseClient, 'get_endpoint', replace_get_endpoint) +def test_get_note_details(*args): + shipyard_client = get_api_client() + note_id = "ABC123ABC123ZZABC123ABC123" + result = shipyard_client.get_note_details(note_id) + assert result['url'] == '{}/notedetails/{}'.format( + shipyard_client.get_endpoint(), note_id) + + @mock.patch.object(BaseClient, 'post_resp', replace_post_rep) @mock.patch.object(BaseClient, 'get_resp', replace_get_resp) @mock.patch.object(BaseClient, 'get_endpoint', replace_get_endpoint) diff --git a/src/bin/shipyard_client/tests/unit/cli/get/test_get_actions.py b/src/bin/shipyard_client/tests/unit/cli/get/test_get_actions.py index 105c35a3..12ffb0e9 100644 --- a/src/bin/shipyard_client/tests/unit/cli/get/test_get_actions.py +++ b/src/bin/shipyard_client/tests/unit/cli/get/test_get_actions.py @@ -104,7 +104,9 @@ def test_get_actions(*args): assert 'Lifecycle' in response assert '2/1/0' in response assert 'This is a note for the concurrency check' not in response - assert '>>> Your lucky numbers are 1, 3, 5, and Q' in response + assert "Action Footnotes" in response + assert (" - Info available with 'describe notedetails/" + "ABCDEFGHIJKLMNOPQRSTUVWXYA'") in response GET_ACTIONS_API_RESP_UNPARSEABLE_NOTE = """