Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc: update examples to reflect alternative ways to provide sudo option #5418

Merged
merged 2 commits into from
Jun 20, 2024

Conversation

ani-sinha
Copy link
Contributor

For creating users and groups, it is possible to pass a sudo option to the
config file that accepts a sudo rule. The option can be a sudo rule string,
a list of sudo rule strings or False to explicitly deny sudo usage. Update
examples to show how a list of strings can be used with sudo option.

@ani-sinha ani-sinha changed the title Fix sudo doc doc: update examples to reflect alternative ways to provide sudo option Jun 20, 2024
Copy link
Member

@holmanb holmanb left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

holmanb and others added 2 commits June 20, 2024 07:38
This configuration:

```
users:
  - name: osadmin
    lock_passwd: false
    sudo: ["ALL=(ALL) NOPASSWD:ALL"]
```

Is valid syntax but is missing from the jsonschema definition.

Fixes canonicalGH-5399
…tion (canonical#5418)

For creating users and groups, it is possible to pass a `sudo` option to the
config file that accepts a sudo rule. The option can be a sudo rule string,
a list of sudo rule strings or `False` to explicitly deny sudo usage. Update
examples to show how a list of strings can be used with `sudo` option.

Signed-off-by: Ani Sinha <[email protected]>
@holmanb
Copy link
Member

holmanb commented Jun 20, 2024

@ani-sinha I just force pushed to this branch to include the PR number in the first line of the individual commits.

@holmanb holmanb merged commit cbcb053 into canonical:main Jun 20, 2024
21 of 22 checks passed
holmanb added a commit that referenced this pull request Jun 20, 2024
This configuration:

```
users:
  - name: osadmin
    lock_passwd: false
    sudo: ["ALL=(ALL) NOPASSWD:ALL"]
```

Is valid syntax but is missing from the jsonschema definition.

Fixes GH-5399
@ani-sinha
Copy link
Contributor Author

ani-sinha commented Jun 26, 2024

@holmanb there is also a broad issue. The schema validator complains that ssh-authorized-keys (and likely other parameters with a - instead of _ ) are invalid. Yet, the ug_util.py has this logic

    # Ensure user options are in the right python friendly format                                                                                                                                 
    if users:
        c_users = {}
        for uname, uconfig in users.items():
            c_uconfig = {}
            for k, v in uconfig.items():
                k = k.replace("-", "_").strip()
                if k:
                    c_uconfig[k] = v
            c_users[uname] = c_uconfig
        users = c_users

Thus - work. So what to do about this? The schema validator can ignore differences between names with- and names with _ with a warning to place it to canonical form.

@holmanb
Copy link
Member

holmanb commented Jun 26, 2024

@ani-sinha I nearly missed this comment. Please open a new issue referencing the closed PR or issue in the future so that things like this don't get missed.

The schema validator complains that ssh-authorized-keys (and likely other parameters with a - instead of _ ) are invalid

Since ssh-authorized-keys is deprecated, I think that it is doing the right thing here (I'd have to see an exact config in order to know, maybe I missed something). However, you're right that this code means that we're missing a couple of keys that technically work but aren't represented in the schema. The ones that I think need a schema entry are: plain_text_passwd, create_groups, selinux_user, primary_group, hashed_passwd, no_log_init, no_create_home, no_user_group.

@ani-sinha
Copy link
Contributor Author

@ani-sinha I nearly missed this comment. Please open a new issue referencing the closed PR or issue in the future so that things like this don't get missed.

The schema validator complains that ssh-authorized-keys (and likely other parameters with a - instead of _ ) are invalid

Since ssh-authorized-keys is deprecated, I think that it is doing the right thing here (I'd have to see an exact config in order to know, maybe I missed something). However, you're right that this code means that we're missing a couple of keys that technically work but aren't represented in the schema. The ones that I think need a schema entry are: plain_text_passwd, create_groups, selinux_user, primary_group, hashed_passwd, no_log_init, no_create_home, no_user_group.

@holmanb I raised #5454 .

holmanb added a commit that referenced this pull request Jun 28, 2024
This configuration:

```
users:
  - name: osadmin
    lock_passwd: false
    sudo: ["ALL=(ALL) NOPASSWD:ALL"]
```

Is valid syntax but is missing from the jsonschema definition.

Fixes GH-5399
holmanb pushed a commit that referenced this pull request Jun 28, 2024
…tion (#5418)

For creating users and groups, it is possible to pass a `sudo` option to the
config file that accepts a sudo rule. The option can be a sudo rule string,
a list of sudo rule strings or `False` to explicitly deny sudo usage. Update
examples to show how a list of strings can be used with `sudo` option.

Signed-off-by: Ani Sinha <[email protected]>
aitorpazos added a commit to aitorpazos/molecule-qemu that referenced this pull request Aug 1, 2024
Ubuntu noble ships a cloud-init version without this fix: canonical/cloud-init#5418
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants