Integration name should be validated

weimeilin79 commented 5 years ago

This is a problem my lab user encounter

They name the integration with "dot" in it, and was not able to run the integration.


[X] Feature request
[ ] Regression (a behavior that used to work and stopped working in a new release)
[X] Bug report  
[ ] Documentation issue or request

Description

When creating integration, can we add an UI validation? I had an user created an integration with dot(".") in it, and it was saved ok, but when hit publish, it was not able to start.

Checked the logs, apparently the server is complaining about the naming, so it was not able to deploy, can you please add an validation in UI so it will never happen again? Thanks

` 2018-12-10 18:12:34.995 ERROR [-,,,] 1 --- [tion Controller] i.s.s.c.integration.online.BaseHandler : Integration [add.location]: [ERROR] Activation failure

| | javax.validation.ConstraintViolationException: Constraint Validations: name must match "^a-z0-9?$" on bean: Container(args=[], command=[], env=[EnvVar(name=LOADER_HOME, value=${JAVA_DATA_DIR}/syndesis/loader, valueFrom=null, additionalProperties={}), EnvVar(name=AB_JMX_EXPORTER_CONFIG, value=/tmp/src/prometheus-config.yml, valueFrom=null, additionalProperties={})], envFrom=[], image=docker-registry.default.svc:5000/fuse-f3668ffe-fc91-11e8-bd1e-0a580a800009/i-add.location:1, imagePullPolicy=Always, lifecycle=null, livenessProbe=null, name=i-add.location, ports=[ContainerPort(containerPort=8778, hostIP=null, hostPort=null, name=jolokia, protocol=null, additionalProperties={})], readinessProbe=null, resources=null, securityContext=null, stdin=null, stdinOnce=null, terminationMessagePath=null, terminationMessagePolicy=null, tty=null, volumeMounts=[VolumeMount(mountPath=/deployments/config, name=secret-volume, readOnly=false, subPath=null, additionalProperties={})], workingDir=null, additionalProperties={}) | at io.fabric8.kubernetes.api.builder.ValidationUtils.validate(ValidationUtils.java:36) ~[kubernetes-model-2.0.10.jar!/:2.0.10] | at io.fabric8.kubernetes.api.model.ContainerBuilder.build(ContainerBuilder.java:81) ~[kubernetes-model-2.0.10.jar!/:2.0.10] | at io.fabric8.kubernetes.api.model.PodSpecFluentImpl$ContainersNestedImpl.and(PodSpecFluentImpl.java:1063) ~[kubernetes-model-2.0.10.jar!/:2.0.10] | at io.fabric8.kubernetes.api.model.PodSpecFluentImpl$ContainersNestedImpl.endContainer(PodSpecFluentImpl.java:1066) ~[kubernetes-model-2.0.10.jar!/:2.0.10] | at io.syndesis.server.openshift.OpenShiftServiceImpl.ensureDeploymentConfig(OpenShiftServiceImpl.java:251) ~[server-openshift-1.4.8.fuse-710001-redhat-00001.jar!/:1.4.8.fuse-710001-redhat-00001] | at io.syndesis.server.openshift.OpenShiftServiceImpl.deploy(OpenShiftServiceImpl.java:85) ~[server-openshift-1.4.8.fuse-710001-redhat-00001.jar!/:1.4.8.fuse-710001-redhat-00001] | at io.syndesis.server.controller.integration.online.PublishHandler.deploy(PublishHandler.java:178) ~[server-controller-1.4.8.fuse-710001-redhat-00001.jar!/:1.4.8.fuse-710001-redhat-00001] | at io.syndesis.server.controller.integration.online.PublishHandler$BuildStepPerformer.perform(PublishHandler.java:329) ~[server-controller-1.4.8.fuse-710001-redhat-00001.jar!/:1.4.8.fuse-710001-redhat-00001] | at io.syndesis.server.controller.integration.online.PublishHandler.execute(PublishHandler.java:114) ~[server-controller-1.4.8.fuse-710001-redhat-00001.jar!/:1.4.8.fuse-710001-redhat-00001] | at io.syndesis.server.controller.StateChangeHandler.execute(StateChangeHandler.java:33) [server-controller-1.4.8.fuse-710001-redhat-00001.jar!/:1.4.8.fuse-710001-redhat-00001] | at io.syndesis.server.controller.integration.IntegrationController.lambda$callStateChangeHandler$11(IntegrationController.java:199) [server-controller-1.4.8.fuse-710001-redhat-00001.jar!/:1.4.8.fuse-710001-redhat-00001] | at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149) ~[na:1.8.0_181] | at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624) ~[na:1.8.0_181] | at java.lang.Thread.run(Thread.java:748) ~[na:1.8.0_181]

`

gashcrumb commented 5 years ago

Thought we were actually mangling the name before using it for actual stuff?

I don't think we should impose a constraint user-supplied names personally, instead they should be sanitized if they're being used for server entities.

zregvart commented 5 years ago

Resolved via #3925

There's a test that contains a .. I also tested on staging with an integration named .integration.name..

https://github.com/syndesisio/syndesis/blob/2c84588b6de0820bf0c20f455bde46d9e7ea2bb4/app/common/util/src/test/java/io/syndesis/common/util/NamesTest.java#L59

syndesisio / syndesis