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

[k8s] Special case project creation so that it is possible #42132

Merged
merged 1 commit into from
Jul 17, 2018

Conversation

fabianvf
Copy link
Contributor

SUMMARY

Automatically translates Projects to ProjectRequests if the user isn't permissioned enough to make Projects. This basically makes project creation for low-permissioned users the same as running oc new-project. This again makes it possible to create projects from Ansible with < admin permissions on the cluster.

Fixes #42116

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

k8s

ADDITIONAL INFORMATION

This should be backported to 2.6

@fabianvf
Copy link
Contributor Author

@maxamillion

@fabianvf fabianvf changed the title Special case project creation so that it is possible [k8s] Special case project creation so that it is possible Jun 29, 2018
@ansibot ansibot added affects_2.7 This issue/PR affects Ansible v2.7 bug This issue/PR relates to a bug. needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Jun 29, 2018
@ansibot
Copy link
Contributor

ansibot commented Jun 29, 2018

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

lib/ansible/module_utils/k8s/raw.py:125:29: E127 continuation line over-indented for visual indent

click here for bot help

@ansibot ansibot added ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jun 29, 2018
@ansibot ansibot removed ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jun 29, 2018
@s-hertel s-hertel removed the needs_triage Needs a first human triage before being processed. label Jun 29, 2018
@fabianvf
Copy link
Contributor Author

fabianvf commented Jul 2, 2018

cc @chouseknecht @flaper87

@maxamillion
Copy link
Contributor

rebuild_merge

@maxamillion
Copy link
Contributor

@fabianvf does this need to be backported to 2.6?

@fabianvf
Copy link
Contributor Author

fabianvf commented Jul 9, 2018

@maxamillion yeah, you can't make projects in 2.6 right now without cluster-level admin permissions

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Jul 9, 2018
@fabianvf
Copy link
Contributor Author

fabianvf commented Jul 9, 2018

@maxamillion can you rekick the build?

@maxamillion
Copy link
Contributor

rebuild_merge

@fabianvf fabianvf force-pushed the projectrequest branch 2 times, most recently from f7f0cc5 to 0ee0b5a Compare July 13, 2018 17:29
@ansibot
Copy link
Contributor

ansibot commented Jul 13, 2018

@ansibot ansibot removed stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jul 13, 2018
@willthames
Copy link
Contributor

Does the get fail only when the project doesn't exist? Otherwise it will presumably try and recreate the project, returning changed=True even if the project already exists?

@fabianvf
Copy link
Contributor Author

Projects in OpenShift are different than most resources, since almost no user has permission to create them. There's a fake resource called ProjectRequest that will allow a user to request a project and depending on cluster configuration a project will be provisioned for that user. A ProjectRequest isn't actually created though, so the user won't have permission to get it. The proper way to idempotently handle project creation for unpermissioned users is to check if the project exists and make a ProjectRequest if it doesn't.

@willthames
Copy link
Contributor

Surely the permission error would then happen on project create, not on project get?

Your description of what the process should be sounds reasonable to me, I'm just not convinced that this change is implementing that process - I'd expect the ForbiddenError to be caught after project creation attempt

@fabianvf
Copy link
Contributor Author

Good point, the forbidden error on get will happen if you create a ProjectRequest but maybe not if you create a Project, I'll check it out and amend the PR tomorrow if necessary.

@fabianvf
Copy link
Contributor Author

@willthames it looks like the current behavior is correct, when I try to GET a project that I don't have access to I get a Forbidden error

@maxamillion
Copy link
Contributor

rebuild_merge

@maxamillion maxamillion merged commit 9eccc96 into ansible:devel Jul 17, 2018
fabianvf added a commit to fabianvf/ansible that referenced this pull request Jul 19, 2018
mattclay pushed a commit that referenced this pull request Jul 27, 2018
@dagwieers dagwieers added the k8s label Feb 8, 2019
@ansible ansible locked and limited conversation to collaborators Jul 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.7 This issue/PR affects Ansible v2.7 bug This issue/PR relates to a bug. k8s support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[k8s] Can't create Projects with low-permissioned users
7 participants