Skip to content

Rename "principals" to "names".#791

Merged
rshriram merged 2 commits intoistio:release-1.1from
liminw:rbac
Feb 11, 2019
Merged

Rename "principals" to "names".#791
rshriram merged 2 commits intoistio:release-1.1from
liminw:rbac

Conversation

@liminw
Copy link
Copy Markdown
Contributor

@liminw liminw commented Feb 9, 2019

Since this is defined under "subjects", we are basically referring to
the "name" of a subject.

Since this is defined under "subjects", we are basically referring to
the "name" of a subject.
@googlebot googlebot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Feb 9, 2019
@liminw liminw requested review from yangminzhu and removed request for andraxylia and costinm February 9, 2019 01:23
Copy link
Copy Markdown
Contributor

@yangminzhu yangminzhu left a comment

Choose a reason for hiding this comment

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

/lgtm

// `source.principal` attribute. If not specified, it applies to any principals.
repeated string principals = 4;
// Optional. A list of subject names. This is matched to the
// `source.principal` attribute. If one of subject names is "*", it matches any subject.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If one of subject names is "*", it matches any subject.
I find this a little confusing, does the second subject refer to the whole subject message (including other fields)? Or does it mean only the names field in the subject?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It only applies to the "names" field in the subject. Other conditions for the subject still needs to be evaluated.

@yangminzhu
Copy link
Copy Markdown
Contributor

/lgtm

@istio-testing
Copy link
Copy Markdown
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: liminw, yangminzhu
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: rshriram

If they are not already assigned, you can assign the PR to them by writing /assign @rshriram in a comment when ready.

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

The pull request process is described 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

@liminw liminw requested review from rshriram and wenchenglu February 9, 2019 01:52
@rshriram rshriram merged commit 27010bf into istio:release-1.1 Feb 11, 2019
@liminw liminw deleted the rbac branch February 11, 2019 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. review/done

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants