refactor(ci): modularize aws.sh with namespace prefixes and deduplication (RHIDP-11714)#4327
Conversation
|
/test e2e-eks-helm-nightly |
|
/test e2e-eks-operator-nightly |
|
/test e2e-eks-helm-nightly |
…tion RHIDP-11714 - Add source-once guard and module description header - Remove dead code: aws_configure(), aws_eks_get_load_balancer_hostname() - Extract _aws::get_hosted_zone_id (4x duplication) and _aws::apply_route53_change (3x duplication) private helpers - Rename all functions with aws::/_aws:: namespace prefixes - Replace fragile cat/sed JSON construction in cleanup with jq - Use mktemp instead of fixed /tmp/ paths to prevent race conditions - Migrate bare echo/echo >&2 to log:: functions, remove emoji from messages - Add input validation to public functions - Update callers in eks-helm.sh and eks-operator.sh Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace fragile $? checks with `if ! var=$(cmd)` pattern in aws::generate_domain_name and aws::get_certificate to prevent accidental return code clobbering. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…xplicitly Convert three remaining $? patterns in aws::get_certificate to idiomatic `if ! var=$(cmd)` style. Refactor aws::configure_ingress_and_dns to accept domain_name as explicit parameter instead of reading EKS_INSTANCE_DOMAIN_NAME global, improving testability and reducing hidden coupling. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…tion Replace 7 duplicated "Missing required parameter" blocks with a single _aws::require_param helper that centralizes validation, error logging, and optional usage hints. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… deployment On EKS, the Crunchy Postgres operator is not installed so the CRD check always times out and fails. The RHDH operator supports both Crunchy PostgresCluster and built-in StatefulSet databases, so the CRD check should be informational rather than a hard failure. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Address review feedback: the CRD is required because the operator relies on CrunchyDB for its internal database. Revert to blocking check with return 1 on failure. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
9d85d22 to
19822b4
Compare
|
/test e2e-eks-helm-nightly |
The showcase-k8s project only excluded token-propagation-workflow but allowed other orchestrator tests to run, which fail because orchestrator infrastructure is never installed on K8s environments. Align with showcase-rbac-k8s which already excludes all orchestrator tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
https://redhat-internal.slack.com/archives/C07D05S8U82/p1772512617636149 |
|
/test e2e-eks-helm-nightly |
|
@gustavolira: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
…uire_param Address SonarQube code smell: positional parameters $1, $2, $3 should be assigned to named local variables for readability. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
|
/lgtm |
80525d5
into
redhat-developer:main
|
/cherry-pick release-1.9 |
|
@zdrapela: #4327 failed to apply on top of branch "release-1.9": DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
| "**/playwright/e2e/dynamic-home-page-customization.spec.ts", | ||
| "**/playwright/e2e/plugins/scorecard/scorecard.spec.ts", | ||
| "**/playwright/e2e/plugins/orchestrator/token-propagation-workflow.spec.ts", | ||
| "**/playwright/e2e/plugins/orchestrator/**/*.spec.ts", |
There was a problem hiding this comment.
@gustavolira can you please backport this PR, or at least this fix to release-1.9?
…from redhat-developer#4327) Backport the playwright config fix from PR redhat-developer#4327 to release-1.9. Adds orchestrator/**/*.spec.ts glob pattern to SHOWCASE_K8S testIgnore to properly exclude all orchestrator tests in K8S test runs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>



Summary
aws.sh(RHIDP-11714): Add source-once guard, remove dead code (aws_configure,aws_eks_get_load_balancer_hostname), extract_aws::get_hosted_zone_id(4x dedup) and_aws::apply_route53_change(3x dedup) helpers, namespace all functions withaws::/_aws::prefixes, migrate bareechotolog::calls, replace fragilecat/sed -iJSON construction in cleanup withjq, usemktempinstead of fixed/tmp/paths, add input validation to public functionseks-helm.shandeks-operator.shto use new namespaced function namesTest plan
cd .ibm && yarn shellcheck— passes with no new warningscd .ibm && yarn prettier:check— formatting passesgrep -r "generate_dynamic_domain_name\|get_eks_certificate\|configure_eks_ingress_and_dns\|cleanup_eks_dns_record" .ibm/pipelines/— returns zero hits (all old names replaced)🤖 Generated with Claude Code