From c5592343c84468ce7b314d11e250c263502ad276 Mon Sep 17 00:00:00 2001 From: bufdev <4228796+bufdev@users.noreply.github.com> Date: Fri, 23 Feb 2024 14:51:38 -0500 Subject: [PATCH 1/2] Move DepRefs and DigestType to top-level (#76) --- .../federation/v1beta1/upload_service.proto | 46 +++++++++++-------- .../module/v1beta1/download_service.proto | 14 +++--- .../module/v1beta1/upload_service.proto | 38 ++++++--------- 3 files changed, 48 insertions(+), 50 deletions(-) diff --git a/buf/registry/legacy/federation/v1beta1/upload_service.proto b/buf/registry/legacy/federation/v1beta1/upload_service.proto index a090574..25e8693 100644 --- a/buf/registry/legacy/federation/v1beta1/upload_service.proto +++ b/buf/registry/legacy/federation/v1beta1/upload_service.proto @@ -41,36 +41,32 @@ service UploadService { // See the package documentation for more details. You should likely use buf.registry.module.v1beta1 // and not this package. message UploadRequest { - // A dependency of Content, either referencing another Content message, or referencing - // a Commit that already exists. + // A dependency of one or more references specified by Contents. + // + // Dependencies between Contents are implicit and do not need to be specified. The BSR will detect + // dependencies between Contents via .proto imports. message DepRef { - // The Module of the dep. - buf.registry.module.v1beta1.ModuleRef module_ref = 1 [(buf.validate.field).required = true]; - // The commit_id of the Commit, if this is referencing a Commit that already exists. - // - // If the ModuleRef refers to a Module that has associated Content, this field should *not* - // be set, and setting it is an error. - string commit_id = 2 [(buf.validate.field).string.uuid = true]; - // The registry hostname of the dep. - string registry = 3 [(buf.validate.field).required = true]; + // The commit_id of the dependency. + string commit_id = 1 [ + (buf.validate.field).required = true, + (buf.validate.field).string.uuid = true + ]; + // The registry hostname of the dependency. + string registry = 2 [(buf.validate.field).required = true]; } // Content to upload for a given reference. message Content { // The Module of the reference. buf.registry.module.v1beta1.ModuleRef module_ref = 1 [(buf.validate.field).required = true]; - // The dependencies of the reference. - // - // This will include all transitive dependencies. - repeated DepRef dep_refs = 2; // The Files of the Content. // // This will consist of the .proto files, license files, and documentation files. - repeated buf.registry.module.v1beta1.File files = 3 [(buf.validate.field).repeated.min_items = 1]; + repeated buf.registry.module.v1beta1.File files = 2 [(buf.validate.field).repeated.min_items = 1]; // The original v1beta1 or v1 buf.yaml file that encapsulated this reference, if it existed. // // This is used in deprecated digest calculations only. None of the structured // information within this File will or should convey further information about the reference. - buf.registry.module.v1beta1.File v1_buf_yaml_file = 4; + buf.registry.module.v1beta1.File v1_buf_yaml_file = 3; // The original v1beta1 or v1 buf.lock file that encapsulated this reference, if it existed. // // This is used in deprecated digest calculations only. None of the structured @@ -78,7 +74,7 @@ message UploadRequest { // // Importantly, this file is *not* used to determine the dependencies of the reference. To // specify the dependencies, use the dep_refs fields. - buf.registry.module.v1beta1.File v1_buf_lock_file = 5; + buf.registry.module.v1beta1.File v1_buf_lock_file = 4; // The labels to associate with the Commit for the Content. // // If an id is set, this id must represent a Label that already exists and is @@ -89,12 +85,12 @@ message UploadRequest { // // If the Labels do not exist, they will be created. // If the Labels were archived, they will be unarchived. - repeated buf.registry.module.v1beta1.ScopedLabelRef scoped_label_refs = 6; + repeated buf.registry.module.v1beta1.ScopedLabelRef scoped_label_refs = 5; // The URL of the source control commit to associate with the Commit for this Content. // // BSR users can navigate to this link to find source control information that is relevant to this Commit // (e.g. commit description, PR discussion, authors, approvers, etc.). - string source_control_url = 7 [ + string source_control_url = 6 [ (buf.validate.field).string.uri = true, (buf.validate.field).string.max_len = 255, (buf.validate.field).ignore = IGNORE_IF_UNPOPULATED @@ -102,6 +98,16 @@ message UploadRequest { } // The Contents of all references. repeated Content contents = 1 [(buf.validate.field).repeated.min_items = 1]; + // The dependencies of the references specified by Contents. + // + // This will include all transitive dependencies. + // + // Dependencies between Contents are implicit and do not need to be specified. The BSR will detect + // dependencies between Contents via .proto imports. + // + // Commits should be unique by Module, that is no two dep_refs should have the same Module but + // different Commit IDs. + repeated DepRef dep_refs = 2; } // See the package documentation for more details. You should likely use buf.registry.module.v1beta1 diff --git a/buf/registry/module/v1beta1/download_service.proto b/buf/registry/module/v1beta1/download_service.proto index 18d929c..8863efa 100644 --- a/buf/registry/module/v1beta1/download_service.proto +++ b/buf/registry/module/v1beta1/download_service.proto @@ -82,19 +82,19 @@ message DownloadRequest { // // If false, it is an error to specify non-existent file paths. bool paths_allow_not_exist = 4; - // The DigestType to return for the Commit of this reference. - // - // If this DigestType is not available, an error is returned. - // Note that certain DigestTypes may be deprecated over time. - // - // If not set, the latest DigestType is used, currently B5. - DigestType digest_type = 5 [(buf.validate.field).enum.defined_only = true]; } // The references to get contents for. repeated Value values = 1 [ (buf.validate.field).repeated.min_items = 1, (buf.validate.field).repeated.max_items = 250 ]; + // The DigestType to return for the Commits of the references. + // + // If this DigestType is not available, an error is returned. + // Note that certain DigestTypes may be deprecated over time. + // + // If not set, the latest DigestType is used, currently B5. + DigestType digest_type = 2 [(buf.validate.field).enum.defined_only = true]; } message DownloadResponse { diff --git a/buf/registry/module/v1beta1/upload_service.proto b/buf/registry/module/v1beta1/upload_service.proto index 5337433..8c35b81 100644 --- a/buf/registry/module/v1beta1/upload_service.proto +++ b/buf/registry/module/v1beta1/upload_service.proto @@ -33,37 +33,19 @@ service UploadService { } message UploadRequest { - // A dependency of Content, either referencing another Content message, or referencing - // a Commit that already exists. - message DepRef { - // The Module of the dep. - ModuleRef module_ref = 1 [(buf.validate.field).required = true]; - // The commit_id of the Commit, if this is referencing a Commit that already exists. - // - // If the ModuleRef refers to a Module that has associated Content, this field should be empty, - // and setting a value is an error. - string commit_id = 2 [ - (buf.validate.field).string.uuid = true, - (buf.validate.field).ignore_empty = true - ]; - } // Content to upload for a given reference. message Content { // The Module of the reference. ModuleRef module_ref = 1 [(buf.validate.field).required = true]; - // The dependencies of the reference. - // - // This will include all transitive dependencies. - repeated DepRef dep_refs = 2; // The Files of the Content. // // This will consist of the .proto files, license files, and documentation files. - repeated File files = 3 [(buf.validate.field).repeated.min_items = 1]; + repeated File files = 2 [(buf.validate.field).repeated.min_items = 1]; // The original v1beta1 or v1 buf.yaml file that encapsulated this reference, if it existed. // // This is used in deprecated digest calculations only. None of the structured // information within this File will or should convey further information about the reference. - File v1_buf_yaml_file = 4; + File v1_buf_yaml_file = 3; // The original v1beta1 or v1 buf.lock file that encapsulated this reference, if it existed. // // This is used in deprecated digest calculations only. None of the structured @@ -71,7 +53,7 @@ message UploadRequest { // // Importantly, this file is *not* used to determine the dependencies of the reference. To // specify the dependencies, use the dep_refs fields. - File v1_buf_lock_file = 5; + File v1_buf_lock_file = 4; // The labels to associate with the Commit for the Content. // // If an id is set, this id must represent a Label that already exists and is @@ -82,12 +64,12 @@ message UploadRequest { // // If the Labels do not exist, they will be created. // If the Labels were archived, they will be unarchived. - repeated ScopedLabelRef scoped_label_refs = 6; + repeated ScopedLabelRef scoped_label_refs = 5; // The URL of the source control commit to associate with the Commit for this Content. // // BSR users can navigate to this link to find source control information that is relevant to this Commit // (e.g. commit description, PR discussion, authors, approvers, etc.). - string source_control_url = 7 [ + string source_control_url = 6 [ (buf.validate.field).string.uri = true, (buf.validate.field).string.max_len = 255, (buf.validate.field).ignore = IGNORE_IF_UNPOPULATED @@ -95,6 +77,16 @@ message UploadRequest { } // The Contents of all references. repeated Content contents = 1 [(buf.validate.field).repeated.min_items = 1]; + // The dependencies of the references specified by Contents. + // + // Dependencies between Contents are implicit and do not need to be specified. The BSR will detect + // dependencies between Contenta via .proto imports. + // + // This will include all transitive dependencies. + // + // Commits should be unique by Module, that is no two dep_commit_ids should have the same Module but + // different Commit IDs. + repeated string dep_commit_ids = 2 [(buf.validate.field).repeated.items.string.uuid = true]; } message UploadResponse { From 641cb69ff6726048032127042bf7fad58c6fe33a Mon Sep 17 00:00:00 2001 From: bufdev <4228796+bufdev@users.noreply.github.com> Date: Mon, 26 Feb 2024 18:34:51 -0500 Subject: [PATCH 2/2] Support policy checks (#69) --- Makefile | 2 +- buf/registry/module/v1beta1/label.proto | 40 +++++++++++++++++++ .../module/v1beta1/label_service.proto | 23 +++++++++-- 3 files changed, 61 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index c9596d2..ca9b775 100644 --- a/Makefile +++ b/Makefile @@ -10,7 +10,7 @@ BIN := .tmp/bin export PATH := $(BIN):$(PATH) export GOBIN := $(abspath $(BIN)) -BUF_VERSION := v1.28.1 +BUF_VERSION := v1.29.0 COPYRIGHT_YEARS := 2023-2024 .PHONY: help diff --git a/buf/registry/module/v1beta1/label.proto b/buf/registry/module/v1beta1/label.proto index d5b7f30..969fda4 100644 --- a/buf/registry/module/v1beta1/label.proto +++ b/buf/registry/module/v1beta1/label.proto @@ -61,6 +61,7 @@ message Label { ]; // The id of the Commit currently associated with the Label. // + // If policy checks are enabled, this will point to the most recent Commit that passed or was approved. // To get the history of the Commits that have been associated with a Label, use ListLabelHistory. string commit_id = 8 [ (buf.validate.field).required = true, @@ -71,6 +72,45 @@ message Label { (buf.validate.field).required = true, (buf.validate.field).string.uuid = true ]; + // The CommitCheckState for the Commit the Label points to. + // + // The CommitCheckStatus will always be disabled, passed, or approved, since Labels will + // never point to pending or rejected Commits. + // + // TODO: Add custom CEL validation to validate the status field is one of DISABLED, PASSED, APPROVED. + CommitCheckState commit_check_state = 10 [(buf.validate.field).required = true]; +} + +// The state of a Commit's policy checks for a particular Label. +// +// Policy checks are an enterprise-only feature - contact us to learn more! +message CommitCheckState { + // The status of the policy check. + CommitCheckStatus status = 1 [ + (buf.validate.field).enum.defined_only = true, + (buf.validate.field).required = true + ]; + // The time the policy check state was last updated. + // + // If the status is disabled, this will be equal to the Commit create_time. + google.protobuf.Timestamp update_time = 3 [(buf.validate.field).required = true]; +} + +// A check status for a Commit. +// +// Policy checks are an enterprise-only feature - contact us to learn more! +enum CommitCheckStatus { + COMMIT_CHECK_STATUS_UNSPECIFIED = 0; + // Policy checks were not enabled when the Commit was created. + COMMIT_CHECK_STATUS_DISABLED = 1; + // The Commit did not fail any policy checks and therefore did not need review. + COMMIT_CHECK_STATUS_PASSED = 2; + // The Commit has not yet been reviewed after failing policy checks and is pending. + COMMIT_CHECK_STATUS_PENDING = 3; + // The Commit was reviewed after failing policy checks and was rejected. + COMMIT_CHECK_STATUS_REJECTED = 4; + // The Commit was reviewed after failing policy checks and was approved. + COMMIT_CHECK_STATUS_APPROVED = 5; } // LabelRef is a reference to a Label, either an id or a fully-qualified name. diff --git a/buf/registry/module/v1beta1/label_service.proto b/buf/registry/module/v1beta1/label_service.proto index 37f751f..4491946 100644 --- a/buf/registry/module/v1beta1/label_service.proto +++ b/buf/registry/module/v1beta1/label_service.proto @@ -96,7 +96,8 @@ message ListLabelsRequest { // Once the resource is resolved, the following Labels are listed: // - If a Module is referenced, all Labels for the Module are returned. // - If a Label is referenced, this Label is returned. - // - If a Commit is referenced, all Labels for the Commit are returned. + // - If a Commit is referenced, all Labels that currently point to the Commit are returned. Note that + // Labels only point to passed or approved Commits, or Commits where policy checks were disabled. ResourceRef resource_ref = 3 [(buf.validate.field).required = true]; // The order to return the Labels. // @@ -106,6 +107,15 @@ message ListLabelsRequest { // TODO: We are purposefully not making the default the zero enum value, however // we may want to consider this. Order order = 4 [(buf.validate.field).enum.defined_only = true]; + // Only return Labels that point to a Commit with one of these CommitCheckStatus values. + // + // If not set, Labels that point to a Commit with any CommitCheckStatus value are returned. + // + // It is an error to filter on CommitCheckStatuses of pending or rejected, as Labels will only + // point to Commits that are passed or approved, or that have policy checks disabled. + // + // TODO: Add custom CEL validation to validate the status field is one of DISABLED, PASSED, APPROVED. + repeated CommitCheckStatus commit_check_statuses = 5 [(buf.validate.field).repeated.items.enum.defined_only = true]; } message ListLabelsResponse { @@ -157,12 +167,19 @@ message ListLabelHistoryRequest { } message ListLabelHistoryResponse { + message Value { + // The Commit. + Commit commit = 1 [(buf.validate.field).required = true]; + // The CommitCheckState for this Commit on this Label. + CommitCheckState commit_check_state = 2 [(buf.validate.field).required = true]; + } + // The next page token. // /// If empty, there are no more pages. string next_page_token = 1 [(buf.validate.field).string.max_len = 4096]; - // The listed Commits that represent the history of the Label. - repeated Commit commits = 2; + // The ordered history of the Label. + repeated Value values = 2; } message CreateOrUpdateLabelsRequest {