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

Add image-auditor role to ManageIQ SA #1737

Merged

Conversation

moolitayer
Copy link
Contributor

@moolitayer moolitayer commented Apr 10, 2016

@openshift-bot
Copy link

Can one of the admins verify this patch?

@openshift-bot
Copy link

Can one of the admins verify this patch?
I understand the following comments:

  • bot, add author to whitelist
  • bot, test pull request
  • bot, test pull request once

@moolitayer
Copy link
Contributor Author

fyi @simon3z @enoodle @deads2k

@deads2k
Copy link
Contributor

deads2k commented Apr 11, 2016

image-auditor doesn't exist yet, but I think adding it is probably a good idea. Can you help determine its permissions? I think it would need

  1. get,list,watch on images to know when new images are added to the internal docker registry
  2. patch (update?) on images indicate the status of images.

Is there anything else it needs?

@deads2k
Copy link
Contributor

deads2k commented Apr 11, 2016

Opened openshift/origin#8455. Please comment on necessary permissions there.

@@ -27,6 +27,7 @@ manage_iq_tasks:
- policy add-role-to-user -n management-infra admin -z management-admin
- policy add-role-to-user -n management-infra management-infra-admin -z management-admin
- policy add-cluster-role-to-user cluster-reader system:serviceaccount:management-infra:management-admin
- policy add-cluster-role-to-user image-auditor system:serviceaccount:management-infra:management-admin
Copy link
Contributor

Choose a reason for hiding this comment

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

This role will need to be conditionalized on openshift.common.version_gte_3_2_or_1_2 since the role doesn't exist prior to that.

Copy link
Contributor Author

@moolitayer moolitayer Apr 27, 2016

Choose a reason for hiding this comment

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

This role will need to be conditionalized on openshift.common.version_gte_3_2_or_1_2 since the role doesn't exist prior to that.

@detiber During testing the upgrade scenario I installed with openshift_pkg_version=-1.2.0 after first installing with openshift_pkg_version=-1.1.6 and found out the role does not exist there:

[root@mtayer-centos7-4 ~]# oc version
oc v1.2.0-rc1
kubernetes v1.2.0-36-g4a3f9c5
[root@mtayer-centos7-4 ~]# oc get clusterrole/system:image-auditor
Error from server: role "system:image-auditor" not found

I can not explain this since when I installed regularly (not upgrade) there was no problem

@detiber
Copy link
Contributor

detiber commented Apr 11, 2016

This will require adding the new role to the 3.2 upgrade in playbooks/common/openshift-cluster/upgrades/v3_1_to_v3_2 as well

@simon3z
Copy link
Contributor

simon3z commented Apr 22, 2016

@moolitayer what's up with this one? It's required ASAP.

@moolitayer
Copy link
Contributor Author

moolitayer commented Apr 24, 2016

This will require adding the new role to the 3.2 upgrade in playbooks/common/openshift-cluster/upgrades/v3_1_to_v3_2 as well

@detiber I'm new to openshift-ansible / ansible. I see that v3_1_to_v3_2/roles links to root roles:

ls -ltr playbooks/common/openshift-cluster/upgrades/v3_1_to_v3_2/roles
lrwxrwxrwx. 1 mtayer mtayer 20 Apr 10 16:46 playbooks/common/openshift-cluster/upgrades/v3_1_to_v3_2/roles -> ../../../../../roles

$ grep manage_iq_openshift_3_2_tasks playbooks/common/openshift-cluster/upgrades/v3_1_to_v3_2/roles/openshift_manageiq/vars/main.yml  # The var I added
manage_iq_openshift_3_2_tasks:

Will that take care of the upgrade scenario?

@moolitayer moolitayer force-pushed the manageiq_sa_to_have_image_scanning_role branch from 3f38248 to 0187b30 Compare April 24, 2016 12:04
@moolitayer moolitayer changed the title [WIP] Add image-auditor role to ManageIQ SA Add image-auditor role to ManageIQ SA Apr 24, 2016
@moolitayer
Copy link
Contributor Author

Address comment from @detiber regarding openshift.common.version_gte_3_2_or_1_2

with_items: "{{manage_iq_openshift_3_2_tasks}}"
register: osmiq_perm_3_2_task
failed_when: osmiq_perm_3_2_task.rc != 0
changed_when: osmiq_perm_3_2_task.rc == 0
Copy link
Contributor Author

@moolitayer moolitayer Apr 24, 2016

Choose a reason for hiding this comment

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

@detiber @deads2k The above is wrong as it state changed even if the role existed but I could not think of anything better:
oadm policy does not tell me if the role existed (exit code for both cases is 0 and no output)
The only thing I could think of is parsing the result of

oc describe clusterPolicyBindings :default

but that does not feel right

Copy link
Contributor

Choose a reason for hiding this comment

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

you might be able to use oc get clusterpolicybindings system:image-auditor with the --template argument to return just the user field. I would have to defer to @abutcher or @deads2k on the syntax for that though.

You might also want to consider passing -o yaml or -o json and then you could use the from_yaml or from_json filter to better interact with the return value as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oc get clusterpolicybindings system:image-auditor does not seem to work for me with
Error from server: clusterpolicybinding "system:image-auditor" not found

@deads2k @abutcher is there currently a way to get this info?

https://trello.com/c/YqfYqKdS/545-8-policy-related-endpoints-need-work seems to be related

Copy link
Contributor

Choose a reason for hiding this comment

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

oc get clusterpolicybindings system:image-auditor does not seem to work for me with
Error from server: clusterpolicybinding "system:image-auditor" not found

@deads2k @abutcher is there currently a way to get this info?

oc get clusterrole/system:image-auditor

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oc get clusterrole/system:image-auditor

Does this not only tell me that the role exists? I need to know if it is associated to the serviceAccounts management-infra/management-admin

Copy link
Contributor

Choose a reason for hiding this comment

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

Does this not only tell me that the role exists? I need to know if it is associated to the serviceAccounts management-infra/management-admin

Why don't you just unconditionally bind it? The operation should be idempotent: oadm policy add-cluster-role-to-user.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why don't you just unconditionally bind it? The operation should be idempotent: oadm policy add-cluster-role-to-user.

Yes I am. Question is what to use for changed_when

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes I am. Question is what to use for changed_when

I don't know what change_when is. You can get the binding using oc get clusterpolicybinding (no name arg). It will list them all (should only be one) and you can search for your item. It will be painful.

We don't guarantee rolebinding names, but oc get clusterrolebinding/<find your name> would probably be pretty stable.

Copy link
Contributor

Choose a reason for hiding this comment

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

@deads2k changed_when is an ansible construct that describes to ansible when to determine if the task should be labeled as 'changed' or 'ok' when run. We would want to only mark it as 'changed' if we are adding the rolebinding and not for all future iterations through the task

@detiber
Copy link
Contributor

detiber commented Apr 25, 2016

@sdodson ptal

@detiber detiber self-assigned this Apr 25, 2016
@moolitayer
Copy link
Contributor Author

@sdodson ping?

@sdodson
Copy link
Member

sdodson commented Apr 26, 2016

@moolitayer for the ugprade scenario I think you can get by with simply adding this role to the list of roles to re-run during upgrades, this looks completely idempotent.

Add the following after line 12 https://github.com/openshift/openshift-ansible/blob/master/playbooks/common/openshift-cluster/upgrades/v3_1_to_v3_2/post.yml#L12
- openshift_manageiq

@moolitayer moolitayer force-pushed the manageiq_sa_to_have_image_scanning_role branch from 0187b30 to cb6ff8a Compare April 27, 2016 11:37
@moolitayer
Copy link
Contributor Author

@sdodson @detiber thanks for the feedback, met some problems testing the upgrade scenario #1815

@moolitayer
Copy link
Contributor Author

@abutcher @detiber @sdodson could someone please direct me to how to run upgrades with openshift-ansible to test the upgrade scenario?

I've tried following https://docs.openshift.com/enterprise/3.0/install_config/upgrades.html but it seems out of date since playbooks/adhoc/upgrades/upgrade.yml does not exists. Updated instructions somewhere?

@sdodson
Copy link
Member

sdodson commented May 2, 2016

You're concerned with v3.1 -> v3.2 only, correct?

ansible-playbook [-i </path/to/inventory/file>] \
    playbooks/byo/openshift-cluster/upgrades/v3_1_to_v3_2/upgrade.yml

3.2 docs haven't been published yet but these should still be relevant, just substitute v3_1_to_v3_2.
https://docs.openshift.com/enterprise/3.1/install_config/upgrading/automated_upgrades.html#upgrading-to-openshift-enterprise-3-0-1

@moolitayer
Copy link
Contributor Author

@sdodson Thank you.
OK on the upgrade scenario[1]

AFAIK this is ready, besides if more attention is needed on the changed_when issue - if so Is grep the direction? (there is no better option I am aware of)

@sdodson @detiber ptal

[1]
Installed 1.1.6
sudo ansible-playbook playbooks/byo/config.yml -v -i my-inventory2.ini
with openshift_pkg_version=-1.1.6 in the inventory under [OSEv3:vars]

[root@mtayer-centos7-4 ~]# oc version
oc v1.1.6
kubernetes v1.2.0-36-g4a3f9c5

then removed openshift_pkg_version=-1.1.6 and upgraded to 2 with
sudo ansible-playbook playbooks/byo/openshift-cluster/upgrades/v3_1_to_v3_2/upgrade.yml -v -i my-inventory2.ini

[root@mtayer-centos7-4 ~]# oc version
oc v1.2.0-rc1
kubernetes v1.2.0-36-g4a3f9c5
[root@mtayer-centos7-4 ~]# oc describe clusterPolicyBindings :default|grep image-auditor -A 4
RoleBinding[system:image-auditor]:           
                            Role:           system:image-auditor
                            Users:          <none>
                            Groups:         <none>
                            ServiceAccounts:    management-infra/management-admin
                            Subjects:       <none>

@detiber
Copy link
Contributor

detiber commented May 2, 2016

@moolitayer unfortunately, I think we do need to solve the changed_when issue. I'm fine with ls | grep if that is the only way currently.

@moolitayer
Copy link
Contributor Author

@moolitayer unfortunately, I think we do need to solve the changed_when issue. I'm fine with ls | grep if that is the only way currently.

@detiber given that existing code uses osmiq_perm_task.rc == 0 for all the other manageiq role tasks are you ok with handling that in a different patch?
(see Configure role/user permissions and manage_iq_tasks)

@detiber
Copy link
Contributor

detiber commented May 3, 2016

@detiber given that existing code uses osmiq_perm_task.rc == 0 for all the other manageiq role tasks are you ok with handling that in a different patch?
(see Configure role/user permissions and manage_iq_tasks)

@moolitayer that's fine.

@detiber
Copy link
Contributor

detiber commented May 3, 2016

ok to test

@detiber
Copy link
Contributor

detiber commented May 3, 2016

aos-ci-test

@openshift-bot
Copy link

cb6ff8a - State: success - Test Context: aos-ci-jenkins/OS_unit_tests - Test Description: all unit tests passed - Logs: https://aos-ci.s3.amazonaws.com/openshift/openshift-ansible/jenkins-openshift-ansible-pre-test-501/cb6ff8aa226e3013b5c258f1f05d921c78238717.txt

@openshift-bot
Copy link

cb6ff8a - State: error - Test Context: aos-ci-jenkins/OS_3.1_containerized - Test Description: openshift-ansible install failed - Logs: https://aos-ci.s3.amazonaws.com/openshift/openshift-ansible/jenkins-openshift-ansible-run-openshift-ansible-906/cb6ff8aa226e3013b5c258f1f05d921c78238717.txt

@detiber
Copy link
Contributor

detiber commented May 9, 2016

aos-ci-test

@openshift-bot
Copy link

6d55d92 - State: success - Test Context: aos-ci-jenkins/OS_unit_tests - Test Description: all unit tests passed - Logs: https://aos-ci.s3.amazonaws.com/openshift/openshift-ansible/jenkins-openshift-ansible-pre-test-531/6d55d92799f40a0f2b9c67ef89802deed22ea34e.txt

@openshift-bot
Copy link

6d55d92 - State: error - Test Context: aos-ci-jenkins/OS_3.1_containerized - Test Description: openshift-ansible install failed - Logs: https://aos-ci.s3.amazonaws.com/openshift/openshift-ansible/jenkins-openshift-ansible-run-openshift-ansible-969/6d55d92799f40a0f2b9c67ef89802deed22ea34e.txt

@openshift-bot
Copy link

6d55d92 - State: success - Test Context: aos-ci-jenkins/OS_3.1_NOT_containerized - Test Description: openshift-ansible install passed - Logs: https://aos-ci.s3.amazonaws.com/openshift/openshift-ansible/jenkins-openshift-ansible-run-openshift-ansible-970/6d55d92799f40a0f2b9c67ef89802deed22ea34e.txt

@openshift-bot
Copy link

6d55d92 - State: success - Test Context: aos-ci-jenkins/OS_3.0_NOT_containerized - Test Description: openshift-ansible install passed - Logs: https://aos-ci.s3.amazonaws.com/openshift/openshift-ansible/jenkins-openshift-ansible-run-openshift-ansible-971/6d55d92799f40a0f2b9c67ef89802deed22ea34e.txt

@openshift-bot
Copy link

6d55d92 - State: success - Test Context: aos-ci-jenkins/OS_3.1_NOT_containerized_e2e_tests - Test Description: all e2e tests passed - Logs: https://aos-ci.s3.amazonaws.com/openshift/openshift-ansible/jenkins-openshift-ansible-run-e2e-tests-341/6d55d92799f40a0f2b9c67ef89802deed22ea34e.txt

@detiber detiber assigned sdodson and unassigned detiber May 9, 2016
@detiber
Copy link
Contributor

detiber commented May 9, 2016

@sdodson any luck tracking down the version test issues?

@sdodson
Copy link
Member

sdodson commented May 10, 2016

@sdodson any luck tracking down the version test issues?

Looks to be fixed via #1871

@detiber
Copy link
Contributor

detiber commented May 10, 2016

@sdodson Thanks!

@simon3z
Copy link
Contributor

simon3z commented May 11, 2016

Ping @moolitayer

@detiber
Copy link
Contributor

detiber commented May 12, 2016

aos-ci-test

@openshift-bot
Copy link

6d55d92 - State: success - Test Context: aos-ci-jenkins/OS_unit_tests - Test Description: all unit tests passed - Logs: https://aos-ci.s3.amazonaws.com/openshift/openshift-ansible/jenkins-openshift-ansible-pre-test-566/6d55d92799f40a0f2b9c67ef89802deed22ea34e.txt

@openshift-bot
Copy link

6d55d92 - State: error - Test Context: aos-ci-jenkins/OS_3.0_NOT_containerized - Test Description: openshift-ansible install failed - Logs: https://aos-ci.s3.amazonaws.com/openshift/openshift-ansible/jenkins-openshift-ansible-run-openshift-ansible-1060/6d55d92799f40a0f2b9c67ef89802deed22ea34e.txt

@openshift-bot
Copy link

6d55d92 - State: error - Test Context: aos-ci-jenkins/OS_3.1_containerized - Test Description: openshift-ansible install failed - Logs: https://aos-ci.s3.amazonaws.com/openshift/openshift-ansible/jenkins-openshift-ansible-run-openshift-ansible-1061/6d55d92799f40a0f2b9c67ef89802deed22ea34e.txt

@openshift-bot
Copy link

6d55d92 - State: error - Test Context: aos-ci-jenkins/OS_3.1_NOT_containerized - Test Description: openshift-ansible install failed - Logs: https://aos-ci.s3.amazonaws.com/openshift/openshift-ansible/jenkins-openshift-ansible-run-openshift-ansible-1062/6d55d92799f40a0f2b9c67ef89802deed22ea34e.txt

@detiber
Copy link
Contributor

detiber commented May 12, 2016

aos-ci-test

@openshift-bot
Copy link

6d55d92 - State: success - Test Context: aos-ci-jenkins/OS_unit_tests - Test Description: all unit tests passed - Logs: https://aos-ci.s3.amazonaws.com/openshift/openshift-ansible/jenkins-openshift-ansible-pre-test-574/6d55d92799f40a0f2b9c67ef89802deed22ea34e.txt

@openshift-bot
Copy link

6d55d92 - State: success - Test Context: aos-ci-jenkins/OS_3.0_NOT_containerized - Test Description: openshift-ansible install passed - Logs: https://aos-ci.s3.amazonaws.com/openshift/openshift-ansible/jenkins-openshift-ansible-run-openshift-ansible-1084/6d55d92799f40a0f2b9c67ef89802deed22ea34e.txt

@openshift-bot
Copy link

6d55d92 - State: success - Test Context: aos-ci-jenkins/OS_3.1_containerized - Test Description: openshift-ansible install passed - Logs: https://aos-ci.s3.amazonaws.com/openshift/openshift-ansible/jenkins-openshift-ansible-run-openshift-ansible-1085/6d55d92799f40a0f2b9c67ef89802deed22ea34e.txt

@openshift-bot
Copy link

6d55d92 - State: success - Test Context: aos-ci-jenkins/OS_3.1_NOT_containerized - Test Description: openshift-ansible install passed - Logs: https://aos-ci.s3.amazonaws.com/openshift/openshift-ansible/jenkins-openshift-ansible-run-openshift-ansible-1086/6d55d92799f40a0f2b9c67ef89802deed22ea34e.txt

@openshift-bot
Copy link

6d55d92 - State: success - Test Context: aos-ci-jenkins/OS_3.1_containerized_e2e_tests - Test Description: all e2e tests passed - Logs: https://aos-ci.s3.amazonaws.com/openshift/openshift-ansible/jenkins-openshift-ansible-run-e2e-tests-386/6d55d92799f40a0f2b9c67ef89802deed22ea34e.txt

@openshift-bot
Copy link

6d55d92 - State: success - Test Context: aos-ci-jenkins/OS_3.1_NOT_containerized_e2e_tests - Test Description: all e2e tests passed - Logs: https://aos-ci.s3.amazonaws.com/openshift/openshift-ansible/jenkins-openshift-ansible-run-e2e-tests-385/6d55d92799f40a0f2b9c67ef89802deed22ea34e.txt

@detiber detiber merged commit f63985e into openshift:master May 12, 2016
@jeremyeder
Copy link
Contributor

Has anyone seen this before? Note this is a re-run of openshift-ansible on top of an existing install (3.2.0.15).

TASK [openshift_manageiq : fail] ***********************************************
skipping: [bkr-hv01.lab.eng.rdu.redhat.com]

TASK [openshift_manageiq : Copy Configuration to temporary conf] ***************
ok: [bkr-hv01.lab.eng.rdu.redhat.com]

TASK [openshift_manageiq : Add Managment Infrastructure project] ***************
ok: [bkr-hv01.lab.eng.rdu.redhat.com]

TASK [openshift_manageiq : Create Admin Service Account] ***********************
ok: [bkr-hv01.lab.eng.rdu.redhat.com]

TASK [openshift_manageiq : Create Image Inspector Service Account] *************
ok: [bkr-hv01.lab.eng.rdu.redhat.com]

TASK [openshift_manageiq : Create Cluster Role] ********************************
ok: [bkr-hv01.lab.eng.rdu.redhat.com]

TASK [openshift_manageiq : Configure role/user permissions] ********************
changed: [bkr-hv01.lab.eng.rdu.redhat.com] => (item=policy add-role-to-user -n management-infra admin -z management-admin)
changed: [bkr-hv01.lab.eng.rdu.redhat.com] => (item=policy add-role-to-user -n management-infra management-infra-admin -z management-admin)
changed: [bkr-hv01.lab.eng.rdu.redhat.com] => (item=policy add-cluster-role-to-user cluster-reader system:serviceaccount:management-infra:management-admin)
changed: [bkr-hv01.lab.eng.rdu.redhat.com] => (item=policy add-scc-to-user privileged system:serviceaccount:management-infra:management-admin)
changed: [bkr-hv01.lab.eng.rdu.redhat.com] => (item=policy add-cluster-role-to-user system:image-puller system:serviceaccount:management-infra:inspector-admin)
changed: [bkr-hv01.lab.eng.rdu.redhat.com] => (item=policy add-scc-to-user privileged system:serviceaccount:management-infra:inspector-admin)

TASK [openshift_manageiq : Configure 3_2 role/user permissions] ****************
failed: [bkr-hv01.lab.eng.rdu.redhat.com] (item=policy add-cluster-role-to-user system:image-auditor system:serviceaccount:management-infra:management-admin) => {"changed": false, "cmd": ["oadm", "policy", "add-cluster-role-to-user", "system:image-auditor", "system:serviceaccount:management-infra:management-admin", "--config=/tmp/manageiq_admin.kubeconfig"], "delta": "0:00:00.223015", "end": "2016-05-19 16:17:07.898180", "failed": true, "failed_when_result": true, "item": "policy add-cluster-role-to-user system:image-auditor system:serviceaccount:management-infra:management-admin", "rc": 1, "start": "2016-05-19 16:17:07.675165", "stderr": "Error from server: role \"system:image-auditor\" not found", "stdout": "", "stdout_lines": [], "warnings": []}

@abutcher
Copy link
Member

@jeremyeder I saw this w/ some of the earlier 3.2 packages and @sdodson told me not to mess with anything less than 3.2.0.20.

@detiber
Copy link
Contributor

detiber commented May 19, 2016

Was 3.2.0.15 ever released externally, or is it an internal only build?

@jeremyeder
Copy link
Contributor

Okay, understood.

Jason: Internal only afaik.
On May 19, 2016 17:10, "Andrew Butcher" [email protected] wrote:

@jeremyeder https://github.com/jeremyeder I saw this w/ some of the
earlier 3.2 packages and @sdodson https://github.com/sdodson told me
not to mess with anything less than 3.2.0.20.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#1737 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants