)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"33db9f0f1e5883266682a02ac457cd068c4e3f06","unresolved":true,"context_lines":[{"line_number":12,"context_line":"If we are going to move to using policy to enforce modifications"},{"line_number":13,"context_line":"to images, we need to discard this layer."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"When the flag is True we also don\u0027t insert the policy layer in the"},{"line_number":16,"context_line":"middle of the stack."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"This patch cleans up the stacking to be a little more modular, and"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"df92cb1a_689fcbdf","line":15,"range":{"start_line":15,"start_character":9,"end_line":15,"end_character":13},"updated":"2021-06-24 05:00:19.000000000","message":"IMO better to add name of flag with default value and how it will be flipped.","commit_id":"19078269a4452ae884409334867a9567da420f96"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"47487f2aa32e661bf3907a966b591a785a3256f9","unresolved":false,"context_lines":[{"line_number":12,"context_line":"If we are going to move to using policy to enforce modifications"},{"line_number":13,"context_line":"to images, we need to discard this layer."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"When the flag is True we also don\u0027t insert the policy layer in the"},{"line_number":16,"context_line":"middle of the stack."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"This patch cleans up the stacking to be a little more modular, and"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"ebef6280_c5af21f0","line":15,"range":{"start_line":15,"start_character":9,"end_line":15,"end_character":13},"in_reply_to":"df92cb1a_689fcbdf","updated":"2021-06-24 15:11:20.000000000","message":"Ack","commit_id":"19078269a4452ae884409334867a9567da420f96"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"33db9f0f1e5883266682a02ac457cd068c4e3f06","unresolved":true,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":"This patch cleans up the stacking to be a little more modular, and"},{"line_number":19,"context_line":"adds a flag to control whether or not we get the authorization layer"},{"line_number":20,"context_line":"on top or not."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Change-Id: Ie8bea199175a0eb35428b1508af1c49111a87c37"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"01ec847d_480eccb9","line":20,"updated":"2021-06-24 05:00:19.000000000","message":"I think we should start tagging\nRelated Blueprint: policy-refactor","commit_id":"19078269a4452ae884409334867a9567da420f96"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"47487f2aa32e661bf3907a966b591a785a3256f9","unresolved":false,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":"This patch cleans up the stacking to be a little more modular, and"},{"line_number":19,"context_line":"adds a flag to control whether or not we get the authorization layer"},{"line_number":20,"context_line":"on top or not."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Change-Id: Ie8bea199175a0eb35428b1508af1c49111a87c37"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"84cbe2dd_bd246480","line":20,"in_reply_to":"01ec847d_480eccb9","updated":"2021-06-24 15:11:20.000000000","message":"Ack","commit_id":"19078269a4452ae884409334867a9567da420f96"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"e9ff547dd1613b71c8960bb444533db1fde109eb","unresolved":true,"context_lines":[{"line_number":9,"context_line":"This makes us able to optionally not get the authorization layer on"},{"line_number":10,"context_line":"top of the stack. The authorization layer makes everything on an"},{"line_number":11,"context_line":"Image read-only based on hard-coded checks for \"owner or admin\"."},{"line_number":12,"context_line":"If we are going to move to using policy to enforce modifications"},{"line_number":13,"context_line":"to images, we need to discard this layer."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"When the flag is True we also don\u0027t insert the policy layer in the"},{"line_number":16,"context_line":"middle of the stack."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"5b4736d2_07df3dc6","line":13,"range":{"start_line":12,"start_character":0,"end_line":13,"end_character":41},"updated":"2021-06-28 17:20:34.000000000","message":"I\u0027m not convinced we actually do want this. There should be no reason why non-owner non-admin should be ever allowed to modify an image. This is one of those things that is safeguarding people from writing bad policies and ending up someone able to modify/delete public/community image or something they are just member of.","commit_id":"44fd0e8eff8b54870dc12d72226798122bb850d9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"10d1b7c1ca437f07106c6491006ee9f1c311bc6a","unresolved":true,"context_lines":[{"line_number":9,"context_line":"This makes us able to optionally not get the authorization layer on"},{"line_number":10,"context_line":"top of the stack. The authorization layer makes everything on an"},{"line_number":11,"context_line":"Image read-only based on hard-coded checks for \"owner or admin\"."},{"line_number":12,"context_line":"If we are going to move to using policy to enforce modifications"},{"line_number":13,"context_line":"to images, we need to discard this layer."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"When the flag is True we also don\u0027t insert the policy layer in the"},{"line_number":16,"context_line":"middle of the stack."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"e7d0a8bf_affbf363","line":13,"range":{"start_line":12,"start_character":0,"end_line":13,"end_character":41},"in_reply_to":"5b4736d2_07df3dc6","updated":"2021-06-28 17:29:32.000000000","message":"This is what we discussed at PTG as necessary for implementing RBAC. I definitely don\u0027t think we should be enforcing (and remain limited to) \"only owner or admin can do anything with an image\" just so that admins don\u0027t make mistakes while writing custom policies.","commit_id":"44fd0e8eff8b54870dc12d72226798122bb850d9"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"02ac9fabc58ec6299daf57e49995d14f90a53163","unresolved":true,"context_lines":[{"line_number":12,"context_line":"If we are going to move to using policy to enforce modifications"},{"line_number":13,"context_line":"to images, we need to discard this layer."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"When the flag is True we also don\u0027t insert the policy layer in the"},{"line_number":16,"context_line":"middle of the stack."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"This patch cleans up the stacking to be a little more modular, and"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"ae5c7357_7b18004c","line":15,"range":{"start_line":15,"start_character":9,"end_line":15,"end_character":21},"updated":"2021-07-01 04:49:09.000000000","message":"will there be any config option to flip this flag or else who and how this will be flipped.","commit_id":"2b9fe30acd4b7fd92782bbe262eb764d245b4482"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"26369d521e5d1d9bfbee5873219412baeadfcc83","unresolved":true,"context_lines":[{"line_number":12,"context_line":"If we are going to move to using policy to enforce modifications"},{"line_number":13,"context_line":"to images, we need to discard this layer."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"When the flag is True we also don\u0027t insert the policy layer in the"},{"line_number":16,"context_line":"middle of the stack."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"This patch cleans up the stacking to be a little more modular, and"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"d4b8170e_6d0f3ef3","line":15,"range":{"start_line":15,"start_character":9,"end_line":15,"end_character":21},"in_reply_to":"ae5c7357_7b18004c","updated":"2021-07-01 13:36:37.000000000","message":"This got kinda lost in our discussion yesterday, but what I said was I hope we can make this not a config flag, but rather a \"this code has been refactored flag. Meaning, once v2.update() is refactored, it starts passing True here to get a repo with no auth/policy layers. When get() is refactored, it does the same, and so on until everyone is passing True and then we can remove it. Your test refactor to run everything with the defaults and my job to make us run the rbac and legacy defaults will help make sure that the flag being static works as I describe (and if not, we\u0027ll have to do something else).","commit_id":"2b9fe30acd4b7fd92782bbe262eb764d245b4482"}],"glance/gateway.py":[{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"e9ff547dd1613b71c8960bb444533db1fde109eb","unresolved":true,"context_lines":[{"line_number":96,"context_line":"                repo, context, property_rules)"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"        if authorization_layer:"},{"line_number":99,"context_line":"            repo \u003d authorization.ImageRepoProxy(repo, context)"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"        return repo"},{"line_number":102,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"90293b76_a46e0a76","line":99,"updated":"2021-06-28 17:20:34.000000000","message":"I think this is potentially very dangerous change to drop. The hard requirement of being owner or admin is removed we\u0027re going to open can of worms that is not easy to clean and getting the read-only repo is good failsafe for anyone implementing changes to really think if they should be doing what they are if this layer is preventing them to proceed.\n\nThis would be very fundamental change to the API (allowing permissive policies to overcome the direct ownership/admin requirement) and I think it should be discussed separately from the discussion of where our policies are enforced.","commit_id":"44fd0e8eff8b54870dc12d72226798122bb850d9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"10d1b7c1ca437f07106c6491006ee9f1c311bc6a","unresolved":true,"context_lines":[{"line_number":96,"context_line":"                repo, context, property_rules)"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"        if authorization_layer:"},{"line_number":99,"context_line":"            repo \u003d authorization.ImageRepoProxy(repo, context)"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"        return repo"},{"line_number":102,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"27622a50_f97f260f","line":99,"in_reply_to":"90293b76_a46e0a76","updated":"2021-06-28 17:29:32.000000000","message":"\u003e I think this is potentially very dangerous change to drop. The hard requirement of being owner or admin is removed we\u0027re going to open can of worms that is not easy to clean and getting the read-only repo is good failsafe for anyone implementing changes to really think if they should be doing what they are if this layer is preventing them to proceed.\n\u003e \n\u003e This would be very fundamental change to the API (allowing permissive policies to overcome the direct ownership/admin requirement) and I think it should be discussed separately from the discussion of where our policies are enforced.\n\nBut see, that\u0027s the whole point of the RBAC work. The fact that the admin role is special is what makes things so inflexible. That\u0027s why other projects have moved away from is_admin to role:admin, and it\u0027s why we\u0027re having to refactor this stuff for system scope as it is. Having this kind of hard-coded \"admin is special\" rule is the same reason unix\u0027s permission model of having to delegate basically everything to root all the time is such a disaster.\n\nIt\u0027s also the reason we had to do all the refactoring of the copy-image process to delegate to admin. The desire was to allow some users to copy images that they can *read* to other stores where they can be used more easily. That involves some trivial modification of the image (by glance, on behalf of an otherwise unprivileged user) in order to do that (i.e. just setting the copying-to-stores fields, etc), but they couldn\u0027t because of this hard and fast enforcement. So, we basically implemented sudo inside of glance for that feature, which is, as you know, complicated.","commit_id":"44fd0e8eff8b54870dc12d72226798122bb850d9"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"3cc335cb87af2218ed9dcf9b2d45500cfad3e78b","unresolved":true,"context_lines":[{"line_number":77,"context_line":"        :param context: The RequestContext"},{"line_number":78,"context_line":"        :param authorization_layer: Controls whether or not we add the legacy"},{"line_number":79,"context_line":"                                    glance.authorization and glance.policy"},{"line_number":80,"context_line":"                                    layers."},{"line_number":81,"context_line":"        :returns: An ImageRepo-like object"},{"line_number":82,"context_line":"        \"\"\""},{"line_number":83,"context_line":"        repo \u003d glance.db.ImageRepo(context, self.db_api)"}],"source_content_type":"text/x-python","patch_set":16,"id":"0f6a2fa9_0edc36d9","line":80,"range":{"start_line":80,"start_character":0,"end_line":80,"end_character":43},"updated":"2021-08-02 17:43:07.000000000","message":"If \"authorization_layer\u003dTrue\" means that we turn on the legacy layers, shouldn\u0027t it default to False? Otherwise in the future, all our calls to get repo will be \"get_repo(context, authorization_layer\u003dFalse)\".\n\nWhile we\u0027re at it, maybe we should even name the parameter \"legacy_authorization_layer\" so make it really really clear that this is a thing of the past.","commit_id":"71ed92a3c9fa6faeef68d680803d5f956ea83273"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"40f684c8467f412035e7b5c86accf29ca5fd2dbd","unresolved":true,"context_lines":[{"line_number":77,"context_line":"        :param context: The RequestContext"},{"line_number":78,"context_line":"        :param authorization_layer: Controls whether or not we add the legacy"},{"line_number":79,"context_line":"                                    glance.authorization and glance.policy"},{"line_number":80,"context_line":"                                    layers."},{"line_number":81,"context_line":"        :returns: An ImageRepo-like object"},{"line_number":82,"context_line":"        \"\"\""},{"line_number":83,"context_line":"        repo \u003d glance.db.ImageRepo(context, self.db_api)"}],"source_content_type":"text/x-python","patch_set":16,"id":"e0f19456_9ed3a4a2","line":80,"range":{"start_line":80,"start_character":0,"end_line":80,"end_character":43},"in_reply_to":"0f6a2fa9_0edc36d9","updated":"2021-08-02 17:48:02.000000000","message":"\u003e If \"authorization_layer\u003dTrue\" means that we turn on the legacy layers, shouldn\u0027t it default to False? Otherwise in the future, all our calls to get repo will be \"get_repo(context, authorization_layer\u003dFalse)\".\n\nIt\u0027s \u003dTrue by default because that keeps the existing behavior. As we convert layers above this, we make those pass \u003dFalse to disable the legacy stuff. After we\u0027re all done, we can remove that layer and all the flags. This is intended as transition, and won\u0027t live forever.\n\n\u003e While we\u0027re at it, maybe we should even name the parameter \"legacy_authorization_layer\" so make it really really clear that this is a thing of the past.\n\nWe could, but since it\u0027s not going to live long term, it\u0027s not a big deal I think. Making it too long gets annoying and at the moment, the legacy stuff is the more common.","commit_id":"71ed92a3c9fa6faeef68d680803d5f956ea83273"}]}