Skip to content

Make Access Request validator private#54660

Merged
kopiczko merged 1 commit intomasterfrom
kopiczko/request_prompt__private_validator
May 13, 2025
Merged

Make Access Request validator private#54660
kopiczko merged 1 commit intomasterfrom
kopiczko/request_prompt__private_validator

Conversation

@kopiczko
Copy link
Copy Markdown
Contributor

@kopiczko kopiczko commented May 9, 2025 •
edited
Loading

Changes:

  • RequestValidator -> requestValidator
  • requestValidator.opts are now typed as a public struct. This is for ValidateAccessRequestForUser godoc
  • made all fields private - there was probably some convention at the beginning but it seems to be lost and it feels more confusing than anything
  • NewRequestValidator -> newRequestValidator
  • GetRequestableRoles -> getRequestableRoles
  • CanSearchAsRole -> canSearchAsRole
  • CanRequestRole -> canRequestRole
  • SystemAnnotations -> systemAnnotations
  • ExpandVars -> WithExpandVars (I'm adding WithDryRun in a subsequent PR and DryRun looks strange)

@kopiczko kopiczko force-pushed the kopiczko/request_prompt__private_validator branch from 1e9c5e8 to d0efce4 Compare May 9, 2025 10:54
@kopiczko kopiczko marked this pull request as ready for review May 9, 2025 10:55
@kopiczko kopiczko added no-changelog Indicates that a PR does not require a changelog entry backport/branch/v17 labels May 9, 2025
@github-actions github-actions Bot requested review from bernardjkim and r0mant May 9, 2025 10:55
@kopiczko kopiczko force-pushed the kopiczko/request_prompt__private_validator branch from d0efce4 to 8204b48 Compare May 9, 2025 12:58
bernardjkim
bernardjkim reviewed May 9, 2025
View reviewed changes
Comment thread lib/services/access_request.go Outdated
Comment thread lib/services/access_request.go Outdated
@kopiczko kopiczko force-pushed the kopiczko/request_prompt__private_validator branch from 8204b48 to 7ee0254 Compare May 9, 2025 18:23
@kopiczko kopiczko requested a review from bernardjkim May 9, 2025 18:24
@kopiczko kopiczko force-pushed the kopiczko/request_prompt__private_validator branch from 7ee0254 to ef61f06 Compare May 9, 2025 18:25
@kopiczko kopiczko mentioned this pull request May 9, 2025
Merged
@kopiczko kopiczko force-pushed the kopiczko/request_prompt__private_validator branch from ef61f06 to 82e2540 Compare May 9, 2025 20:33
@kopiczko
Copy link
Copy Markdown
Contributor Author

kopiczko commented May 9, 2025

Also renamed ExpandVars -> WithExpandVars like a typical option, because I'm adding WithDryRun in #54642

@kopiczko kopiczko force-pushed the kopiczko/request_prompt__private_validator branch from 82e2540 to e2de975 Compare May 9, 2025 20:34
@kopiczko kopiczko requested a review from smallinsky May 12, 2025 08:43
@public-teleport-github-review-bot public-teleport-github-review-bot Bot removed the request for review from r0mant May 13, 2025 09:05
@kopiczko kopiczko enabled auto-merge May 13, 2025 12:59
@kopiczko kopiczko force-pushed the kopiczko/request_prompt__private_validator branch from e2de975 to bd9d2a4 Compare May 13, 2025 12:59
@kopiczko kopiczko added this pull request to the merge queue May 13, 2025
Merged via the queue into master with commit c7dac12 May 13, 2025
39 of 40 checks passed
@kopiczko kopiczko deleted the kopiczko/request_prompt__private_validator branch May 13, 2025 20:55
@backport-bot-workflows
Copy link
Copy Markdown
Contributor

backport-bot-workflows Bot commented May 13, 2025

@kopiczko See the table below for backport results.

Branch Result
branch/v17 Create PR

@backport-bot-workflows
Copy link
Copy Markdown
Contributor

backport-bot-workflows Bot commented May 13, 2025

@kopiczko See the table below for backport results.

Branch Result
branch/v16 Failed
branch/v17 Failed

@kopiczko kopiczko mentioned this pull request May 13, 2025
Merged
@kopiczko
Copy link
Copy Markdown
Contributor Author

kopiczko commented May 14, 2025 •
edited
Loading

There will be no backport to v16. There are too many missing features in the request validator (e.g. capabilities calculation, reason required).

@kopiczko kopiczko mentioned this pull request May 27, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bernardjkim bernardjkim bernardjkim approved these changes

@smallinsky smallinsky smallinsky approved these changes

Assignees

No one assigned

Labels

backport/branch/v17 no-changelog Indicates that a PR does not require a changelog entry size/sm

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@kopiczko @bernardjkim @smallinsky