Add write-kubeconfig-group flag to server

[kittydoor]

Proposed Changes

Add the flag as discussed in issue #9209 in order to enable configuring of group for writing kubeconfig.

Types of Changes

New Feature

Verification

Manually run k3s server with the flag --write-kubeconfig-group and check if kubeconfig has appropriate groups.

Testing

It seems there is no relevant test I found for write-kubeconfig-mode (other than it incidentally being used in some tests. If there is a request for tests please point me in the right direction!

Linked Issues

• New k3s server flag: --write-kubeconfig-own or --write-kubeconfig-group #9209

User-Facing Change

Yes, it creates a new optional flag.

New flag in k3s server: --write-kubeconfig-group

Further Comments

Unfortunately, my laptop doesn't have Linux on it at the moment (due to some suspend issues with new hardware support I'm procrastinating), and under WSL2 I'm having trouble running the final binary to test whether this commit works. Once I have time again I will try this in a VM or on baremetal linux and update here with any changes needed.

[brandond]

https://systemd.io/USER_NAMES/ seems like a good reference document

In strict mode, only uppercase and lowercase characters are allowed, as well as digits, underscores and hyphens. The first character may not be a digit or hyphen.

I would suggest:

1. Attempt to parse the value as an integer
2. If the value can be parsed as an integer, attempt to look it up as a numeric group ID
3. If the group ID lookup succeeded, use it as the group
4. If step 1 or 2 failed, attempt to look up the value as a name.
5. If step 4 failed, return an error.

If someone has a group with a numeric name, that conflicts with the ID of a different group, that is a problem they will have to solve on their own. Preventing that is why most sane systems require group names to start with a letter.

[kittydoor]

Awesome! 👍 I don't know enough about BSD land or Apple land, and who knows other OS' or unique distros, but generally speaking as long as we are saying known issue / goes against common ground rules for naming so we won't take into consideration, I'm happy to implement the magic.

I'll update this PR with your suggestions @brandond

[brandond]

If the value can be parsed as an integer, attempt to look it up as a numeric group ID

Now that I think about this, perhaps we should skip looking up numeric IDs - chown for example allows you to use numeric IDs that don't resolve to a valid user or group. So lets say, if it can be parsed as a valid int, just use that as the numeric ID.

[kittydoor]

Apologies for the delay, life got in the way. I've modified the feature as requested.

If there are any style changes, better organization across files, or whatever else, please let me know. I'll try and get to it asap :)

This should also be documented on the website and perhaps other places? Also, some test to cover this code would be helpful, no idea what is preferred in actual implementation of this though. #itworksonmymachine

[codecov]

Codecov Report

Attention: Patch coverage is 5.55556% with 17 lines in your changes are missing coverage. Please review.

Project coverage is 43.56%. Comparing base (94e29e2) to head (d22ddd9).
Report is 1 commits behind head on master.

Files	Patch %	Lines
pkg/util/file.go	0.00%	11 Missing ⚠️
pkg/server/server.go	0.00%	3 Missing and 1 partial ⚠️
pkg/kubectl/main.go	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #9233      +/-   ##
==========================================
- Coverage   50.07%   43.56%   -6.51%     
==========================================
  Files         158      158              
  Lines       14032    14049      +17     
==========================================
- Hits         7027     6121     -906     
- Misses       5676     6764    +1088     
+ Partials     1329     1164     -165     

Flag	Coverage Δ
e2etests	36.41% <5.55%> (-10.17%)	⬇️
inttests	37.05% <5.55%> (-0.10%)	⬇️
unittests	16.56% <0.00%> (-0.02%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[kittydoor]

@brandond a friendly hello :) Is this PR waiting for anything from my side, or just low-prio so waiting till you or someone else can get around to it?

[brandond]

We are in an extended code freeze due to overlapping release cycles. We will merge non essential things again soon, probably sometime this coming week.