Add bigtable authorizedview types, mappers, CRD and fuzzer #4026
Conversation
|
Please fix the presubmit failure |
gemmahou
force-pushed
the
resource-bigtable-authorizedview
branch
from
9adf7cd to
f85a2a2
Compare
|
/assign @yuwenma |
gemmahou
force-pushed
the
resource-bigtable-authorizedview
branch
from
f85a2a2 to
6afd3f3
Compare
| // Row prefixes to be included in the AuthorizedView. | ||
| // To provide access to all rows, include the empty string as a prefix (""). | ||
| // +kcc:proto:field=google.bigtable.admin.v2.AuthorizedView.SubsetView.row_prefixes | ||
| RowPrefixes [][]byte `json:"rowPrefixes,omitempty"` |
There was a problem hiding this comment.
Could you confirm if this should be string or []byte? It seems to be very user unfriendly to use []byte
There was a problem hiding this comment.
This is generated and the proto uses [][]byte.
Some thoughts here: 1) we do support []byte type in other resource. I guess it's fine to support a list of []byte as well. 2) the API doc, the field should be a "base64-encoded string of bytes". If change to string I'm afraid user would mistakenly configure this field as regular string, but we can always leverage the underlying API to validate values&process the request.
WDYT?
There was a problem hiding this comment.
It is very hard to configure [][]byte. This seems to be a bad API design if the purpose is for base64-encryption
| @@ -312,6 +319,15 @@ func Visit(msgPath string, msg protoreflect.Message, setter func(v protoreflect. | |||
| visitor.VisitPrimitive(path+"[]", el, setter) | |||
| } | |||
|
|
|||
| case protoreflect.BytesKind: | |||
There was a problem hiding this comment.
Defer to the question about if we should change []byte to string, we may not want this type extension
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: yuwenma The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/lgtm please fix the merge conflict |
gemmahou
force-pushed
the
resource-bigtable-authorizedview
branch
from
8a711cb to
00292f5
Compare
|
/lgtm |
google-oss-prow
bot
merged commit d8c0d86
into
GoogleCloudPlatform:master
Change description
Fixes #
Tests you have done
make ready-prto ensure this PR is ready for review.