-
Notifications
You must be signed in to change notification settings - Fork 42
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
Add proto validation for entity type in ProvidersService #4903
Conversation
string entity = 2; | ||
string entity = 2 [ | ||
(buf.validate.field).string = { | ||
pattern: "^[a-z]+(_[a-z]+)*$", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like these string enum values are capitalized?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@evankanderson - Where is that, because I based it on these mappings here -
// Entity types as string-like enums. Used in CLI and other user-facing code
const (
// RepositoryEntity is a repository entity
RepositoryEntity EntityType = "repository"
// BuildEnvironmentEntity is a build environment entity
BuildEnvironmentEntity EntityType = "build_environment"
// ArtifactEntity is an artifact entity
ArtifactEntity EntityType = "artifact"
// PullRequestEntity is a pull request entity
PullRequestEntity EntityType = "pull_request"
// ReleaseEntity is an entity abstracting a release
ReleaseEntity EntityType = "release"
// PipelineRunEntity is an entity abstracting a pipeline run (eg a workflow)
PipelineRunEntity EntityType = "pipeline_run"
// TaskRunEntity is an entity abstracting a task run (eg a step)
TaskRunEntity EntityType = "task_run"
// BuildEntity is an entity that represents a software build
BuildEntity EntityType = "build"
// UnknownEntity is an explicitly unknown entity
UnknownEntity EntityType = "unknown"
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, I mis-read this line and the generated entity constants:
It does look like we force these to lower-case on the server side before comparison, but I think this probably fine to start with.
baae081
to
a8a9df0
Compare
string entity = 2; | ||
string entity = 2 [ | ||
(buf.validate.field).string = { | ||
pattern: "^[a-z]+(_[a-z]+)*$", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, I mis-read this line and the generated entity constants:
It does look like we force these to lower-case on the server side before comparison, but I think this probably fine to start with.
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
a8a9df0
to
c3c2595
Compare
Summary
Ref https://github.com/stacklok/minder-stories/issues/94
Change Type
Mark the type of change your PR introduces:
Testing
Outline how the changes were tested, including steps to reproduce and any relevant configurations.
Attach screenshots if helpful.
Review Checklist: