dannycjones commented on code in PR #4499: URL: https://github.com/apache/hadoop/pull/4499#discussion_r909585560
########## hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/delegation_tokens.md: ########## @@ -184,18 +184,18 @@ running in secure mode; somewhere where the `core-site.xml` configuration sets `hadoop.security.authentication` to to `kerberos` or another valid Review Comment: duplicate 'to' in this line ```suggestion sets `hadoop.security.authentication` to `kerberos` or another valid ``` ########## hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/delegation_tokens.md: ########## @@ -413,7 +413,7 @@ The XML settings needed to enable session tokens are: ``` A JSON role policy for the role/session will automatically be generated which will -consist of +consist of: Review Comment: Add a new line below this too, otherwise markdown won't format it properly. (It is broken before your change) ```suggestion consist of: ``` ########## hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/delegation_tokens.md: ########## @@ -108,7 +108,7 @@ password-protected data opaque to clients; they contain the secrets needed to access the relevant S3 buckets and associated services. They are obtained by requesting a delegation token from the S3A filesystem client. -Issued token mey be included in job submissions, passed to running applications, +Issued tokens may be included in job submissions, passed to running applications, etc. This token is specific to an individual bucket; all buckets which a client wishes to work with must have a separate delegation token issued. Review Comment: I find this confusing in the context of delegated role tokens, where the token is _literally_ specific to an individual bucket. Can we make this clearer while we are editing this paragraph? ```suggestion Issued tokens may be included in job submissions, passed to running applications, etc. Delegation tokens are specific to an S3A filesystem. Each filesystem a client wishes to work with must have a separate delegation token issued, regardless of the delegation token variant. ``` Is it correct to describe them specific to a filesystem rather than the bucket? ########## hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/delegation_tokens.md: ########## @@ -37,7 +37,7 @@ the S3A client from the AWS STS service. They have a limited duration so restrict how long an application can access AWS on behalf of a user. Clients with this token have the full permissions of the user. -*Role Delegation Tokens:* These contain an "STS Session Token" requested by by the +*Role Delegation Tokens:* These contain an "STS Session Token" requested by the STS "Assume Role" API, so grant the caller to interact with S3 as specific AWS role, *with permissions restricted to purely accessing that specific S3 bucket*. Review Comment: Maybe we can make it a bit clearer. It utilises a feature of STS allowing you to scope-down permissions which you can pass on. ```suggestion *Role Delegation Tokens:* These contain an "STS Session Token" requested by the STS "Assume Role" API, granting the caller permission to interact with S3 using a specific IAM role, *with permissions restricted to accessing a specific S3 bucket*. ``` ########## hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/delegation_tokens.md: ########## @@ -55,13 +55,13 @@ see [S3A Delegation Token Architecture](delegation_token_architecture.html). ## <a name="background"></a> Background: Hadoop Delegation Tokens. -A Hadoop Delegation Token are is a byte array of data which is submitted to -a Hadoop services as proof that the caller has the permissions to perform +A Hadoop Delegation Token is a byte array of data which is submitted to +Hadoop services as proof that the caller has the permissions to perform the operation which it is requesting — -and which can be passed between applications to *delegate* those permission. +and which can be passed between applications to *delegate* those permissions. -Tokens are opaque to clients, clients who simply get a byte array -of data which they must to provide to a service when required. +Tokens are opaque to clients. Clients simply get a byte array +of data which they must provide to a service when required. Review Comment: Since we are touching this line, add a line break before "Clients simply" to keep sentence diffs independent in future. ########## hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/delegation_tokens.md: ########## @@ -184,18 +184,18 @@ running in secure mode; somewhere where the `core-site.xml` configuration sets `hadoop.security.authentication` to to `kerberos` or another valid authentication mechanism. -* Without enabling security at this level, delegation tokens will not +*Without enabling security at this level, delegation tokens will not be collected.* -Once Kerberos enabled, the process for acquiring tokens is as follows: +Once Kerberos is enabled, the process for acquiring tokens is as follows: 1. Enable Delegation token support by setting `fs.s3a.delegation.token.binding` to the classname of the token binding to use. to use. Review Comment: drop line 194? ```suggestion 1. Enable Delegation token support by setting `fs.s3a.delegation.token.binding` to the classname of the token binding to use. ``` ########## hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/delegation_tokens.md: ########## @@ -141,10 +141,10 @@ for specifics details on the (current) token lifespan. ### <a name="role-tokens"></a> S3A Role Delegation Tokens -A Role Delegation Tokens is created by asking the AWS +A Role Delegation Token is created by asking the AWS [Security Token Service](http://docs.aws.amazon.com/STS/latest/APIReference/Welcome.html) -for set of "Assumed Role" credentials, with a AWS account specific role for a limited duration.. -This role is restricted to only grant access the S3 bucket and all KMS keys, +for set of "Assumed Role" credentials, with an AWS account specific role for a limited duration. +This role is restricted to only grant access the S3 bucket and all KMS keys. Review Comment: We can be more explicit. ```suggestion The resulting session credentials are restricted to grant access to all KMS keys, and to the specific S3 bucket. ``` ########## hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/delegation_tokens.md: ########## @@ -141,10 +141,10 @@ for specifics details on the (current) token lifespan. ### <a name="role-tokens"></a> S3A Role Delegation Tokens -A Role Delegation Tokens is created by asking the AWS +A Role Delegation Token is created by asking the AWS [Security Token Service](http://docs.aws.amazon.com/STS/latest/APIReference/Welcome.html) -for set of "Assumed Role" credentials, with a AWS account specific role for a limited duration.. -This role is restricted to only grant access the S3 bucket and all KMS keys, +for set of "Assumed Role" credentials, with an AWS account specific role for a limited duration. Review Comment: Perhaps we can simplify. ```suggestion for a set of "Assumed Role" session credentials with a limited lifetime, belonging to a given IAM Role. ``` ########## hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/delegation_tokens.md: ########## @@ -465,7 +465,7 @@ that of the role itself: 1h by default, though this can be changed to 12h [In the IAM Console](https://console.aws.amazon.com/iam/home#/roles), or from the AWS CLI. -*Without increasing the duration of role, one hour is the maximum value; +Without increasing the duration of role, one hour is the maximum value; Review Comment: ```suggestion Without increasing the duration of the role, one hour is the maximum value; ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
