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

Validate when --ticket_id is supplied #162

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

krishvoor
Copy link
Member

This PR needs #159

  1. Retrieve the Secret from the AWS Secret Manager
  2. Validate issue_id from jira

1) Retreive the Secret from the AWS Secret Manager
2) Validate issue_id from jira

Signed-off-by: Krishna Harsha Voora <[email protected]>
@krishvoor krishvoor changed the title Validate Issue_ID when --ticket_id is supplied Validate when --ticket_id is supplied Jun 21, 2023
This PR retrieves jira_api from the AWS Secrets Manager
uses that token to verify a Jira Issue ID exists, if exists
tags hypershift clusters accordingly.

Signed-off-by: Krishna Harsha Voora <[email protected]>
@krishvoor krishvoor marked this pull request as ready for review June 22, 2023 12:46
@@ -1091,6 +1095,12 @@ def main():
type=str,
help='Specify a desired branch of the corresponding git',
default='master')
parser.add_argument(
Copy link

@smalleni smalleni Jun 23, 2023

Choose a reason for hiding this comment

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

This is covered by #159. I say we get #159 in and then you can add the validation separately.

Copy link
Member Author

@krishvoor krishvoor Jun 23, 2023

Choose a reason for hiding this comment

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

So, per the description here: we def. need #159 first, I did this so I can avoid post rebase-related changes :P

Copy link
Collaborator

Choose a reason for hiding this comment

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

we need that the ticket will be verified during the argument verification. If we wait until _build_cluster, we will have a lot of stuff already created.

please add a type=check_function and check that the ticket is valid during the argparse phase

Copy link
Member Author

Choose a reason for hiding this comment

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

Ack, will update the code

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated, ptal

@@ -1091,6 +1095,12 @@ def main():
type=str,
help='Specify a desired branch of the corresponding git',
default='master')
parser.add_argument(
Copy link
Collaborator

Choose a reason for hiding this comment

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

we need that the ticket will be verified during the argument verification. If we wait until _build_cluster, we will have a lot of stuff already created.

please add a type=check_function and check that the ticket is valid during the argparse phase

@@ -534,6 +535,9 @@ def _build_cluster(ocm_cmnd, rosa_cmnd, cluster_name_seed, must_gather_all, prov
if operator_roles_prefix:
cluster_cmd.append("--operator-roles-prefix")
cluster_cmd.append(cluster_name_seed)
if args.ticket_id:
jira_apis.verify_issue_id(args.ticket_id)
cluster_cmd.append("--tags=TicketId:{}.format(args.ticket_id")
Copy link
Collaborator

Choose a reason for hiding this comment

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

we need to add here some check, so if ticket id exists, we need to verify the result of verify_issue_id before appending the tag to the cluster_cmd

Copy link
Member Author

Choose a reason for hiding this comment

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

we need to add here some check, so if ticket id exists, we need to verify the result of verify_issue_id before appending the tag to the cluster_cmd

We have a check here, in case of invalid Ticket-Id, it will exit.

'--ticket_id',
type=str,
help='Approved Ticket ID for cloud-spend',
required=False
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we are going to add this, required has to be True

Copy link
Member Author

Choose a reason for hiding this comment

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

If we are going to add this, the required has to be True

We need this ticket-id if our initial funding is exhausted.
As such, I think this should be False

Copy link
Member Author

Choose a reason for hiding this comment

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

Looping in @smalleni @dry923 ^^^ can you weigh in as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hi 👋 @dry923 @smalleni ptal..

Signed-off-by: Krishna Harsha Voora <[email protected]>
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