syndesisio / syndesis-rest

The API for Syndesis - a flexible, customizable, cloud-hosted platform that provides core integration capabilities as a service. It leverages Red Hat's existing product architecture using OpenShift Online/Dedicated and Fuse Integration Services.
https://syndesis-staging.b6ff.rh-idev.openshiftapps.com/api/v1/
Apache License 2.0
6 stars 17 forks source link

Username in label needs to be sanitised (or limitation should be global) #632

Closed rhuss closed 7 years ago

rhuss commented 7 years ago

Currently on staging:

2017-09-22 15:36:34.739 ERROR [-,,,] 1 --- [pool-4-thread-1] i.s.c.integration.IntegrationController  : Error while processing integration status for integration -KueDv5eg4G12FoCmGQi

io.fabric8.kubernetes.client.KubernetesClientException: Failure executing: GET at: https://syndesis-openshift-proxy.syndesis-staging.svc/oapi/v1/namespaces/syndesis-staging/deploymentconfigs?labelSelector=USERNAME%3Drhuss@redhat.com. Message: unable to parse requirement: invalid label value: "rhuss@redhat.com": must match the regex (([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])? (e.g. 'MyValue' or 'my_value' or '12345'). Received status: Status(apiVersion=v1, code=500, details=null, kind=Status, message=unable to parse requirement: invalid label value: "rhuss@redhat.com": must match the regex (([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])? (e.g. 'MyValue' or 'my_value' or '12345'), metadata=ListMeta(resourceVersion=null, selfLink=null, additionalProperties={}), reason=null, status=Failure, additionalProperties={}).
    at io.fabric8.kubernetes.client.dsl.base.OperationSupport.requestFailure(OperationSupport.java:332) ~[kubernetes-client-2.2.13.jar!/:na]
    at io.fabric8.kubernetes.client.dsl.base.OperationSupport.assertResponseCode(OperationSupport.java:271) ~[kubernetes-client-2.2.13.jar!/:na]
    at io.fabric8.kubernetes.client.dsl.base.OperationSupport.handleResponse(OperationSupport.java:241) ~[kubernetes-client-2.2.13.jar!/:na]
    at io.fabric8.kubernetes.client.dsl.base.OperationSupport.handleResponse(OperationSupport.java:234) ~[kubernetes-client-2.2.13.jar!/:na]
    at io.fabric8.kubernetes.client.dsl.base.BaseOperation.list(BaseOperation.java:575) ~[kubernetes-client-2.2.13.jar!/:na]
    at io.fabric8.kubernetes.client.dsl.base.BaseOperation.list(BaseOperation.java:70) ~[kubernetes-client-2.2.13.jar!/:na]
    at io.syndesis.openshift.OpenShiftServiceImpl.lambda$getDeploymentsByLabel$5(OpenShiftServiceImpl.java:105) ~[openshift-0.1-SNAPSHOT.jar!/:0.1-SNAPSHOT]
    at io.fabric8.kubernetes.client.WithRequestCallable.call(WithRequestCallable.java:35) ~[kubernetes-client-2.2.13.jar!/:na]
    at io.syndesis.openshift.OpenShiftServiceImpl.getDeploymentsByLabel(OpenShiftServiceImpl.java:104) ~[openshift-0.1-SNAPSHOT.jar!/:0.1-SNAPSHOT]
    at io.syndesis.controllers.integration.online.ActivateHandler.countDeployments(ActivateHandler.java:210) ~[controllers-0.1-SNAPSHOT.jar!/:0.1-SNAPSHOT]
    at io.syndesis.controllers.integration.online.ActivateHandler.execute(ActivateHandler.java:101) ~[controllers-0.1-SNAPSHOT.jar!/:0.1-SNAPSHOT]
    at io.syndesis.controllers.integration.IntegrationController.lambda$callStatusChangeHandler$10(IntegrationController.java:175) ~[controllers-0.1-SNAPSHOT.jar!/:0.1-SNAPSHOT]
    at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511) ~[na:1.8.0_131]
    at java.util.concurrent.FutureTask.run(FutureTask.java:266) ~[na:1.8.0_131]
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142) ~[na:1.8.0_131]
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617) ~[na:1.8.0_131]
    at java.lang.Thread.run(Thread.java:748) ~[na:1.8.0_131]

which means that the username cannot uncoditionally used as label.

However the bigger issue is, that this is used to count the objects per syndesis user to enforce a limit. The limit which we are needing however is a global limit so that you can not have more than e.g. 1 integration per namespace / project. So the actual fix should be to use a global limit.

zregvart commented 7 years ago

Would it be OK to use the hash of the username or do we need the username to be legible?

rhuss commented 7 years ago

hash would be ok (if its sane)

iocanel commented 7 years ago

Why use a hash and not something that is readable. We already have Names.sanitizeName() which should be enough for this.