)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":28329,"name":"yangjianfeng","display_name":"JeffYang","email":"yjf1970231893@gmail.com","username":"yangjianfeng"},"change_message_id":"2d3f5816bc6c613fbc14eb30659278b4da5a249f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"caf329ee_c97633bb","updated":"2022-03-22 09:51:21.000000000","message":"Just for PoC.","commit_id":"e1ea87702b57507bad3c31052b0ecbcb338d37be"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"ecf7afae799fe570e42f3ccf8ac365975048c81e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"7568c10d_db5b7dfa","updated":"2022-04-05 11:45:31.000000000","message":"One thing I\u0027m concerned about is that kuryr-daemon would need to have RBAC for Node update. That\u0027s quite a lot. We\u0027re doing that now, but it\u0027s not the best design.","commit_id":"65afb677dc02871e219ecf4bd643a243f1493ba0"},{"author":{"_account_id":28329,"name":"yangjianfeng","display_name":"JeffYang","email":"yjf1970231893@gmail.com","username":"yangjianfeng"},"change_message_id":"530ae82b4efd8bf0bdef6e473bdf7129dc33bbf9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"8d99943e_458d81ed","in_reply_to":"7568c10d_db5b7dfa","updated":"2022-04-06 11:38:47.000000000","message":"I\u0027m so sorry, maybe I don\u0027t understand your meads. How should we do this? Do we have any spec for this? In my opinion the k8s service account that the kuryr-daemon used should be created at deployment phase, so we just need to add some explanations in install docs.","commit_id":"65afb677dc02871e219ecf4bd643a243f1493ba0"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"d52786026a41f2d0a7775ed2dea87133a41f9517","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"3a60a07d_31f7cc4a","updated":"2022-06-07 16:48:47.000000000","message":"A bit preliminary review, but publishing my thoughts. I\u0027ll try to finish reviewing tomorrow.","commit_id":"918f3516c15531b5d7af22ad2a3eb8c74833cf73"},{"author":{"_account_id":28329,"name":"yangjianfeng","display_name":"JeffYang","email":"yjf1970231893@gmail.com","username":"yangjianfeng"},"change_message_id":"062f1473b627c596a9271c0cdc15d40eee1eb426","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"a971102b_5b0369b5","in_reply_to":"3a60a07d_31f7cc4a","updated":"2022-06-11 02:33:30.000000000","message":"Thank you very much for you review. I will look for solutions of these issues and commit a new patch asap.","commit_id":"918f3516c15531b5d7af22ad2a3eb8c74833cf73"}],"kuryr_kubernetes/cni/handlers.py":[{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"d52786026a41f2d0a7775ed2dea87133a41f9517","unresolved":true,"context_lines":[{"line_number":125,"context_line":"                k_const.K8S_ANNOTATION_HOST_VIF_OBJ)"},{"line_number":126,"context_line":"            vif_state \u003d node[\u0027metadata\u0027][\u0027annotations\u0027].get("},{"line_number":127,"context_line":"                k_const.K8S_ANNOTATION_HOST_VIF_STATE)"},{"line_number":128,"context_line":"            if not hostVif or vif_state \u003d\u003d \u0027pluged\u0027:"},{"line_number":129,"context_line":"                return"},{"line_number":130,"context_line":"            vif \u003d obj_vif.base.VersionedObject.obj_from_primitive("},{"line_number":131,"context_line":"                jsonutils.loads(hostVif))"}],"source_content_type":"text/x-python","patch_set":18,"id":"b50ef5b2_ef20313b","line":128,"range":{"start_line":128,"start_character":0,"end_line":128,"end_character":52},"updated":"2022-06-07 16:48:47.000000000","message":"What happens if node got rebooted? We should probably still re-plug the VIF, right? But that won\u0027t happen as the annotation is already set to \"pluged\". There\u0027s a typo BTW, this should be \"plugged\".","commit_id":"918f3516c15531b5d7af22ad2a3eb8c74833cf73"},{"author":{"_account_id":28329,"name":"yangjianfeng","display_name":"JeffYang","email":"yjf1970231893@gmail.com","username":"yangjianfeng"},"change_message_id":"062f1473b627c596a9271c0cdc15d40eee1eb426","unresolved":false,"context_lines":[{"line_number":125,"context_line":"                k_const.K8S_ANNOTATION_HOST_VIF_OBJ)"},{"line_number":126,"context_line":"            vif_state \u003d node[\u0027metadata\u0027][\u0027annotations\u0027].get("},{"line_number":127,"context_line":"                k_const.K8S_ANNOTATION_HOST_VIF_STATE)"},{"line_number":128,"context_line":"            if not hostVif or vif_state \u003d\u003d \u0027pluged\u0027:"},{"line_number":129,"context_line":"                return"},{"line_number":130,"context_line":"            vif \u003d obj_vif.base.VersionedObject.obj_from_primitive("},{"line_number":131,"context_line":"                jsonutils.loads(hostVif))"}],"source_content_type":"text/x-python","patch_set":18,"id":"e35de450_9e610926","line":128,"range":{"start_line":128,"start_character":0,"end_line":128,"end_character":52},"in_reply_to":"b50ef5b2_ef20313b","updated":"2022-06-11 02:33:30.000000000","message":"Ack","commit_id":"918f3516c15531b5d7af22ad2a3eb8c74833cf73"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"d52786026a41f2d0a7775ed2dea87133a41f9517","unresolved":true,"context_lines":[{"line_number":168,"context_line":"        instance_info \u003d obj_vif.instance_info.InstanceInfo("},{"line_number":169,"context_line":"                uuid\u003dnode[\u0027metadata\u0027][\u0027uid\u0027], name\u003dnode[\u0027metadata\u0027][\u0027name\u0027])"},{"line_number":170,"context_line":"        b_base.disconnect(vif, instance_info,  \u0027kuryr-host-node\u0027)"},{"line_number":171,"context_line":"        self.k8s_client.patch_node_annotations("},{"line_number":172,"context_line":"                utils.get_node_name(),"},{"line_number":173,"context_line":"                k_const.K8S_ANNOTATION_HOST_VIF_STATE.replace(\u0027/\u0027, \u0027~1\u0027),"},{"line_number":174,"context_line":"                \"unpluged\")"},{"line_number":175,"context_line":""},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"class CNIPipeline(k_dis.EventPipeline):"}],"source_content_type":"text/x-python","patch_set":18,"id":"b4d5ea94_a9df6335","line":174,"range":{"start_line":171,"start_character":0,"end_line":174,"end_character":27},"updated":"2022-06-07 16:48:47.000000000","message":"This is a bit worrying, there is a whitepaper published recently [1] that advises against DaemonSets having permissions to patch/update nodes. The point is that if someone executes container escape on a node, they can find such token and e.g. force node eviction until another pod being the target lands on compromised node. It would be good to find a way of doing that without these permissions. One would be to just have controller watch Nodes and create a CRD that\u0027ll hold the node\u0027s VIF and state.\n\n[1] https://www.paloaltonetworks.com/resources/whitepapers/kubernetes-privilege-escalation-excessive-permissions-in-popular-platforms","commit_id":"918f3516c15531b5d7af22ad2a3eb8c74833cf73"},{"author":{"_account_id":28329,"name":"yangjianfeng","display_name":"JeffYang","email":"yjf1970231893@gmail.com","username":"yangjianfeng"},"change_message_id":"062f1473b627c596a9271c0cdc15d40eee1eb426","unresolved":false,"context_lines":[{"line_number":168,"context_line":"        instance_info \u003d obj_vif.instance_info.InstanceInfo("},{"line_number":169,"context_line":"                uuid\u003dnode[\u0027metadata\u0027][\u0027uid\u0027], name\u003dnode[\u0027metadata\u0027][\u0027name\u0027])"},{"line_number":170,"context_line":"        b_base.disconnect(vif, instance_info,  \u0027kuryr-host-node\u0027)"},{"line_number":171,"context_line":"        self.k8s_client.patch_node_annotations("},{"line_number":172,"context_line":"                utils.get_node_name(),"},{"line_number":173,"context_line":"                k_const.K8S_ANNOTATION_HOST_VIF_STATE.replace(\u0027/\u0027, \u0027~1\u0027),"},{"line_number":174,"context_line":"                \"unpluged\")"},{"line_number":175,"context_line":""},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"class CNIPipeline(k_dis.EventPipeline):"}],"source_content_type":"text/x-python","patch_set":18,"id":"0e8721f0_4a0a4041","line":174,"range":{"start_line":171,"start_character":0,"end_line":174,"end_character":27},"in_reply_to":"b4d5ea94_a9df6335","updated":"2022-06-11 02:33:30.000000000","message":"Ack","commit_id":"918f3516c15531b5d7af22ad2a3eb8c74833cf73"}],"kuryr_kubernetes/controller/handlers/node.py":[{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"ecf7afae799fe570e42f3ccf8ac365975048c81e","unresolved":true,"context_lines":[{"line_number":105,"context_line":"                direction\u003d\u0027ingress\u0027, remote_ip_prefix\u003dself.cluster_cidr,"},{"line_number":106,"context_line":"                project_id\u003dself.project_id)"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"    def on_present(self, node, *args, **kwargs):"},{"line_number":109,"context_line":"        host_vif \u003d node[\u0027metadata\u0027]["},{"line_number":110,"context_line":"            \u0027annotations\u0027].get(\u0027kuryr.openstack.org/hostVif\u0027)"},{"line_number":111,"context_line":"        if host_vif:"}],"source_content_type":"text/x-python","patch_set":3,"id":"8fbb87a4_4f4dccc0","line":108,"range":{"start_line":108,"start_character":8,"end_line":108,"end_character":18},"updated":"2022-04-05 11:45:31.000000000","message":"We\u0027re missing on_finalize() here, nodes can get deleted as well.","commit_id":"65afb677dc02871e219ecf4bd643a243f1493ba0"},{"author":{"_account_id":28329,"name":"yangjianfeng","display_name":"JeffYang","email":"yjf1970231893@gmail.com","username":"yangjianfeng"},"change_message_id":"530ae82b4efd8bf0bdef6e473bdf7129dc33bbf9","unresolved":true,"context_lines":[{"line_number":105,"context_line":"                direction\u003d\u0027ingress\u0027, remote_ip_prefix\u003dself.cluster_cidr,"},{"line_number":106,"context_line":"                project_id\u003dself.project_id)"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"    def on_present(self, node, *args, **kwargs):"},{"line_number":109,"context_line":"        host_vif \u003d node[\u0027metadata\u0027]["},{"line_number":110,"context_line":"            \u0027annotations\u0027].get(\u0027kuryr.openstack.org/hostVif\u0027)"},{"line_number":111,"context_line":"        if host_vif:"}],"source_content_type":"text/x-python","patch_set":3,"id":"bef1a7fa_7a424acb","line":108,"range":{"start_line":108,"start_character":8,"end_line":108,"end_character":18},"in_reply_to":"8fbb87a4_4f4dccc0","updated":"2022-04-06 11:38:47.000000000","message":"Yep, you are right. I will add it at the following patch.","commit_id":"65afb677dc02871e219ecf4bd643a243f1493ba0"},{"author":{"_account_id":28329,"name":"yangjianfeng","display_name":"JeffYang","email":"yjf1970231893@gmail.com","username":"yangjianfeng"},"change_message_id":"97bbe0ffc9007c0b6f71f16e0b48c5424de6ce55","unresolved":false,"context_lines":[{"line_number":105,"context_line":"                direction\u003d\u0027ingress\u0027, remote_ip_prefix\u003dself.cluster_cidr,"},{"line_number":106,"context_line":"                project_id\u003dself.project_id)"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"    def on_present(self, node, *args, **kwargs):"},{"line_number":109,"context_line":"        host_vif \u003d node[\u0027metadata\u0027]["},{"line_number":110,"context_line":"            \u0027annotations\u0027].get(\u0027kuryr.openstack.org/hostVif\u0027)"},{"line_number":111,"context_line":"        if host_vif:"}],"source_content_type":"text/x-python","patch_set":3,"id":"fd6ae73f_c0c7a6b9","line":108,"range":{"start_line":108,"start_character":8,"end_line":108,"end_character":18},"in_reply_to":"bef1a7fa_7a424acb","updated":"2022-05-07 11:07:16.000000000","message":"Done","commit_id":"65afb677dc02871e219ecf4bd643a243f1493ba0"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"d52786026a41f2d0a7775ed2dea87133a41f9517","unresolved":true,"context_lines":[{"line_number":93,"context_line":"        # Add some host routes (Their destinations are the pod subnet pool\u0027s"},{"line_number":94,"context_line":"        # prefixes) to host subnet, in order the host node can forward the"},{"line_number":95,"context_line":"        # traffic that access pod to pod router."},{"line_number":96,"context_line":"        for prefix in self.subnetpool.prefixes:"},{"line_number":97,"context_line":"            expected_host_route \u003d {"},{"line_number":98,"context_line":"                \u0027destination\u0027: prefix, \u0027nexthop\u0027: subnet.gateway_ip}"},{"line_number":99,"context_line":"            if expected_host_route not in existing_host_routes:"},{"line_number":100,"context_line":"                existing_host_routes.append(expected_host_route)"},{"line_number":101,"context_line":"            LOG.info(\"Add host routes %s to host subnet.\", expected_host_route)"},{"line_number":102,"context_line":"        self.net_client.update_subnet("},{"line_number":103,"context_line":"            self.subnet_id, host_routes\u003dexisting_host_routes)"},{"line_number":104,"context_line":"        # Create security group for host tap device"}],"source_content_type":"text/x-python","patch_set":18,"id":"2c5a80bc_fdae551d","line":101,"range":{"start_line":96,"start_character":0,"end_line":101,"end_character":79},"updated":"2022-06-07 16:48:47.000000000","message":"This might be race-condition-prone, worth to double-check what happens when you try to add the same route twice.","commit_id":"918f3516c15531b5d7af22ad2a3eb8c74833cf73"},{"author":{"_account_id":28329,"name":"yangjianfeng","display_name":"JeffYang","email":"yjf1970231893@gmail.com","username":"yangjianfeng"},"change_message_id":"062f1473b627c596a9271c0cdc15d40eee1eb426","unresolved":false,"context_lines":[{"line_number":93,"context_line":"        # Add some host routes (Their destinations are the pod subnet pool\u0027s"},{"line_number":94,"context_line":"        # prefixes) to host subnet, in order the host node can forward the"},{"line_number":95,"context_line":"        # traffic that access pod to pod router."},{"line_number":96,"context_line":"        for prefix in self.subnetpool.prefixes:"},{"line_number":97,"context_line":"            expected_host_route \u003d {"},{"line_number":98,"context_line":"                \u0027destination\u0027: prefix, \u0027nexthop\u0027: subnet.gateway_ip}"},{"line_number":99,"context_line":"            if expected_host_route not in existing_host_routes:"},{"line_number":100,"context_line":"                existing_host_routes.append(expected_host_route)"},{"line_number":101,"context_line":"            LOG.info(\"Add host routes %s to host subnet.\", expected_host_route)"},{"line_number":102,"context_line":"        self.net_client.update_subnet("},{"line_number":103,"context_line":"            self.subnet_id, host_routes\u003dexisting_host_routes)"},{"line_number":104,"context_line":"        # Create security group for host tap device"}],"source_content_type":"text/x-python","patch_set":18,"id":"4b86bc4b_8374af59","line":101,"range":{"start_line":96,"start_character":0,"end_line":101,"end_character":79},"in_reply_to":"2c5a80bc_fdae551d","updated":"2022-06-11 02:33:30.000000000","message":"Ack","commit_id":"918f3516c15531b5d7af22ad2a3eb8c74833cf73"}],"kuryr_kubernetes/linux_net_utils.py":[{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"d52786026a41f2d0a7775ed2dea87133a41f9517","unresolved":true,"context_lines":[{"line_number":58,"context_line":""},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"def add_iptables_chain(table, chain_name):"},{"line_number":61,"context_line":"    cmd \u003d [\u0027iptables\u0027, \u0027-t\u0027,  table, \u0027-N\u0027, chain_name]"},{"line_number":62,"context_line":"    try:"},{"line_number":63,"context_line":"        return processutils.execute(*cmd, run_as_root\u003dTrue)"},{"line_number":64,"context_line":"    except Exception as e:"}],"source_content_type":"text/x-python","patch_set":18,"id":"7deada08_f3c64dbb","line":61,"range":{"start_line":61,"start_character":0,"end_line":61,"end_character":54},"updated":"2022-06-07 16:48:47.000000000","message":"I wonder how all of these play with iftables? It\u0027s fine if it doesn\u0027t, but we got to document that.\n\nOh, kube-proxy would need iptables anyway?","commit_id":"918f3516c15531b5d7af22ad2a3eb8c74833cf73"},{"author":{"_account_id":28329,"name":"yangjianfeng","display_name":"JeffYang","email":"yjf1970231893@gmail.com","username":"yangjianfeng"},"change_message_id":"062f1473b627c596a9271c0cdc15d40eee1eb426","unresolved":true,"context_lines":[{"line_number":58,"context_line":""},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"def add_iptables_chain(table, chain_name):"},{"line_number":61,"context_line":"    cmd \u003d [\u0027iptables\u0027, \u0027-t\u0027,  table, \u0027-N\u0027, chain_name]"},{"line_number":62,"context_line":"    try:"},{"line_number":63,"context_line":"        return processutils.execute(*cmd, run_as_root\u003dTrue)"},{"line_number":64,"context_line":"    except Exception as e:"}],"source_content_type":"text/x-python","patch_set":18,"id":"ed1d10d4_d729a147","line":61,"range":{"start_line":61,"start_character":0,"end_line":61,"end_character":54},"in_reply_to":"7deada08_f3c64dbb","updated":"2022-06-11 02:33:30.000000000","message":"Sorry, I don\u0027t understand this sentence. What\u0027s iftables? Why we should got to document that? And, Where should we document that?","commit_id":"918f3516c15531b5d7af22ad2a3eb8c74833cf73"}],"setup.cfg":[{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"d52786026a41f2d0a7775ed2dea87133a41f9517","unresolved":true,"context_lines":[{"line_number":120,"context_line":"    test_handler \u003d kuryr_kubernetes.tests.unit.controller.handlers.test_fake_handler:TestHandler"},{"line_number":121,"context_line":"    kuryrport \u003d kuryr_kubernetes.controller.handlers.kuryrport:KuryrPortHandler"},{"line_number":122,"context_line":"    openshift_machine \u003d kuryr_kubernetes.controller.handlers.machine:MachineHandler"},{"line_number":123,"context_line":"    node \u003d kuryr_kubernetes.controller.handlers.node:NodeHandler"},{"line_number":124,"context_line":""},{"line_number":125,"context_line":"kuryr_kubernetes.controller.drivers.multi_vif \u003d"},{"line_number":126,"context_line":"    noop \u003d kuryr_kubernetes.controller.drivers.multi_vif:NoopMultiVIFDriver"}],"source_content_type":"text/x-ttcn-cfg","patch_set":18,"id":"5e4bbdf9_cedc8fe6","line":123,"range":{"start_line":123,"start_character":0,"end_line":123,"end_character":64},"updated":"2022-06-07 16:48:47.000000000","message":"Name like \"Node\" is a bit vague and doesn\u0027t explain that it aims at using kube-proxy as an alternative. I think I\u0027d favor something like kube_proxy_node and KubeProxyNodeHandler. But maybe we\u0027ll come up with something better, at the moment that\u0027s a tiny issue.","commit_id":"918f3516c15531b5d7af22ad2a3eb8c74833cf73"},{"author":{"_account_id":28329,"name":"yangjianfeng","display_name":"JeffYang","email":"yjf1970231893@gmail.com","username":"yangjianfeng"},"change_message_id":"062f1473b627c596a9271c0cdc15d40eee1eb426","unresolved":true,"context_lines":[{"line_number":120,"context_line":"    test_handler \u003d kuryr_kubernetes.tests.unit.controller.handlers.test_fake_handler:TestHandler"},{"line_number":121,"context_line":"    kuryrport \u003d kuryr_kubernetes.controller.handlers.kuryrport:KuryrPortHandler"},{"line_number":122,"context_line":"    openshift_machine \u003d kuryr_kubernetes.controller.handlers.machine:MachineHandler"},{"line_number":123,"context_line":"    node \u003d kuryr_kubernetes.controller.handlers.node:NodeHandler"},{"line_number":124,"context_line":""},{"line_number":125,"context_line":"kuryr_kubernetes.controller.drivers.multi_vif \u003d"},{"line_number":126,"context_line":"    noop \u003d kuryr_kubernetes.controller.drivers.multi_vif:NoopMultiVIFDriver"}],"source_content_type":"text/x-ttcn-cfg","patch_set":18,"id":"10e3e7ba_364ebf3a","line":123,"range":{"start_line":123,"start_character":0,"end_line":123,"end_character":64},"in_reply_to":"5e4bbdf9_cedc8fe6","updated":"2022-06-11 02:33:30.000000000","message":"Yep, good idea. Thank you for you suggestion.","commit_id":"918f3516c15531b5d7af22ad2a3eb8c74833cf73"}]}