[SPARK-30797][SQL] Set tradition user/group/other permission to ACL entries when setting up ACLs in truncate table

[viirya]

What changes were proposed in this pull request?

This is a follow-up to the PR #26956. In #26956, the patch proposed to preserve path permission when truncating table. When setting up original ACLs, we need to set user/group/other permission as ACL entries too, otherwise if the path doesn't have default user/group/other ACL entries, ACL API will complain an error Invalid ACL: the user, group and other entries are required..

In short this change makes sure:

1. Permissions for user/group/other are always kept into ACLs to work with ACL API.
2. Other custom ACLs are still kept after TRUNCATE TABLE ([SPARK-30312][SQL] Preserve path permission and acl when truncate table #26956 did this).

Why are the changes needed?

Without this fix, TRUNCATE TABLE will get an error when setting up ACLs if there is no default default user/group/other ACL entries.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Update unit test.

Manual test on dev Spark cluster.

Set ACLs for a table path without default user/group/other ACL entries:

hdfs dfs -setfacl --set 'user:liangchi:rwx,user::rwx,group::r--,other::r--' /user/hive/warehouse/test.db/test_truncate_table

hdfs dfs -getfacl /user/hive/warehouse/test.db/test_truncate_table
# file: /user/hive/warehouse/test.db/test_truncate_table
# owner: liangchi
# group: supergroup
user::rwx
user:liangchi:rwx
group::r--
mask::rwx
other::r--

Then run sql("truncate table test.test_truncate_table"), it works by normally truncating the table and preserve ACLs.

[viirya]

cc @cloud-fan @dongjoon-hyun This targets both mater and branch-3.0.

[cloud-fan]

LGTM as a bug fix, but we should really revist the TRUNCATE TABLE implementation to think about how to make it simpler...

[SparkQA]

Test build #118288 has finished for PR 27548 at commit b2fa454.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[viirya]

@cloud-fan That's good suggestion. Let's see if we can simplify it in next versions.

[dongjoon-hyun]

+1, LGTM. (Pending Jenkins)
Thank you, @viirya , @HyukjinKwon , @cloud-fan .

[dongjoon-hyun]

Oh, @viirya . During deleting my comments, it seems that your reply also is gone. Sorry for that.

[viirya]

@dongjoon-hyun It's ok. Thanks for review!

[dongjoon-hyun]

I'll merge this since the last commit is only comment-only change due to me.
Merged to master/3.0.

[dongjoon-hyun]

Since this is a security stuff and SPARK-30312 is in branch-2.4, I'll merge this to branch-2.4 together after testing locally.

[viirya]

@dongjoon-hyun Ok. Thanks!

[SparkQA]

Test build #118321 has finished for PR 27548 at commit 7568db7.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.