Skip to content

chore: use dynamic delimiter for advanced authorization values#2105

Open
tolusha wants to merge 3 commits intomainfrom
CRW-10255
Open

chore: use dynamic delimiter for advanced authorization values#2105
tolusha wants to merge 3 commits intomainfrom
CRW-10255

Conversation

@tolusha
Copy link
Contributor

@tolusha tolusha commented Mar 23, 2026

What does this PR do?

These changes fix a bug where usernames or group names containing commas would break the advanced authorization config, since commas were hardcoded as the delimiter. Now the operator automatically picks a safe delimiter (from ,, |, ;, :, #, \t) that doesn't conflict with the actual values, and communicates the chosen delimiter to the server via a new env var. It also allows manual delimiter override through ExtraProperties and returns
an error if no safe delimiter can be found.

Screenshot/screencast of this PR

N/A

What issues does this PR fix or reference?

https://issues.redhat.com/browse/CRW-10255

Related to

eclipse-che/che-server#967

How to test this PR?

N/A

Common Test Scenarios

  • Deploy Eclipse Che
  • Start an empty workspace
  • Open terminal and build/run an image
  • Stop a workspace
  • Check operator logs for reconciliation errors or infinite reconciliation loops

PR Checklist

As the author of this Pull Request I made sure that:

Reviewers

Reviewers, please comment how you tested the PR when approving it.

Signed-off-by: Anatolii Bazko <abazko@redhat.com>
@tolusha
Copy link
Contributor Author

tolusha commented Mar 23, 2026

/retest

}

func FindAvailableDelimiter(s string) string {
delimiters := []string{",", "|", ";", ":", "#", "\t"}
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps we should move the delimiter list to a package-level constant for better visibility and reuse.
Also, since the order defines the precedence of delimiter selection, it might be worth documenting this behavior explicitly.

@openshift-ci
Copy link

openshift-ci bot commented Mar 24, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: rohanKanojia, tolusha

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

Details 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

Signed-off-by: Anatolii Bazko <abazko@redhat.com>
@openshift-ci openshift-ci bot removed the lgtm label Mar 24, 2026
@openshift-ci
Copy link

openshift-ci bot commented Mar 24, 2026

New changes are detected. LGTM label has been removed.

@tolusha
Copy link
Contributor Author

tolusha commented Mar 24, 2026

/retest

2 similar comments
@tolusha
Copy link
Contributor Author

tolusha commented Mar 24, 2026

/retest

@tolusha
Copy link
Contributor Author

tolusha commented Mar 25, 2026

/retest

Signed-off-by: Anatolii Bazko <abazko@redhat.com>
@openshift-ci
Copy link

openshift-ci bot commented Mar 25, 2026

@tolusha: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/v19-devworkspace-happy-path a86bdc0 link true /test v19-devworkspace-happy-path

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

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.

2 participants