Skip to content

Conversation

@zdrapela
Copy link
Member

@zdrapela zdrapela commented Sep 3, 2025

User description

Description

Which issue(s) does this PR fix or relate to

PR acceptance criteria

  • Tests
  • Documentation

How to test changes / Special notes to the reviewer

Summary by Sourcery

Remove legacy command-line options and their handling from prepare-restricted-environment.sh script

Enhancements:

  • Remove deprecated production operator and mirror registry CLI flags (--prod_operator_* and related)
  • Remove version filtering logic for the --prod_operator_version flag

PR Type

Enhancement


Description

  • Remove deprecated command-line options from script

  • Clean up legacy production operator flags

  • Eliminate version filtering logic for old parameters


Diagram Walkthrough

flowchart LR
  A["Legacy CLI Options"] -- "Remove" --> B["Cleaned Script"]
  C["--prod_operator_*"] -- "Delete" --> B
  D["Version Filtering Logic"] -- "Remove" --> B
Loading

File Walkthrough

Relevant files
Enhancement
prepare-restricted-environment.sh
Remove legacy command-line options and handlers                   

.rhdh/scripts/prepare-restricted-environment.sh

  • Remove deprecated --prod_operator_* command-line flags
  • Delete legacy mirror registry storage options
  • Remove version filtering logic for --prod_operator_version
  • Clean up backward compatibility code
+0/-35   

@openshift-ci
Copy link

openshift-ci bot commented Sep 3, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign kadel for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@zdrapela
Copy link
Member Author

zdrapela commented Sep 3, 2025

/review

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey there - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@zdrapela
Copy link
Member Author

zdrapela commented Sep 3, 2025

/review

@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Sep 3, 2025

PR Reviewer Guide 🔍

(Review updated until commit d576f09)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Behavior Change

Removing legacy flags (e.g., --prod_operator_*) eliminates backward compatibility; ensure callers, docs, and CI jobs no longer rely on these options and that helpful error messaging exists if they are still passed.

while [[ "$#" -gt 0 ]]; do
  case $1 in
  # New options
  '--index-image')
    INDEX_IMAGE="$2"
    shift 1
Unused Variable

FILTER_VERSIONS_PROVIDED remains initialized but the handling for version filtering was removed; verify it is still needed or remove to avoid confusion.

FILTER_VERSIONS_PROVIDED="false"

# example usage:
# ./prepare-restricted-environment.sh \
# [ --filter-versions "1.a,1.b" ]

@zdrapela
Copy link
Member Author

zdrapela commented Sep 5, 2025

/scan_repo_discussions

@qodo-code-review
Copy link
Contributor

🚀 I've created a PR with an auto-generated best practices file:
#1592

@qodo-merge-for-open-source
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Backward Compatibility

Removal of legacy flags without shim handling may break existing users or automation still passing these options; confirm that external docs and callers have been updated or provide deprecation messaging.

while [[ "$#" -gt 0 ]]; do
  case $1 in
  # New options
  '--index-image')
    INDEX_IMAGE="$2"
    shift 1
Silent Behavior Change

Previously, --prod_operator_version parsed and set FILTERED_VERSIONS; with its removal, ensure downstream logic no longer relies on FILTER_VERSIONS_PROVIDED/FILTERED_VERSIONS to avoid unexpected defaults.

FILTER_VERSIONS_PROVIDED="false"

# example usage:
# ./prepare-restricted-environment.sh \
# [ --filter-versions "1.a,1.b" ]
# --from-dir /path/to/dir (to support mirroring from a bastion host)
# --to-dir /path/to/dir (to support exporting images to a dir, which can be transferred to the bastion host)
# --to-registry "$MY_MIRROR_REGISTRY" (either this or to-dir needs to specified, both can be specified)
# --install-operator "true"
# --use-oc-mirror "false"

while [[ "$#" -gt 0 ]]; do
  case $1 in
  # New options
  '--index-image')

@zdrapela
Copy link
Member Author

/review
--pr_reviewer.inline_code_comments=false

@qodo-merge-for-open-source
Copy link

PR Code Suggestions ✨

No code suggestions found for the PR.

@qodo-merge-for-open-source
Copy link

Persistent review updated to latest commit d576f09

@zdrapela
Copy link
Member Author

/compliance

@qodo-merge-for-open-source
Copy link

qodo-merge-for-open-source bot commented Sep 10, 2025

PR Compliance Guide 🔍

(Compliance updated until commit d576f09)

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
No custom compliance provided

Follow the guide to enable custom compliance check.

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Previous compliance checks

Compliance check up to commit d576f09
Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
No custom compliance provided

Follow the guide to enable custom compliance check.

@zdrapela
Copy link
Member Author

/compliance

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant