-
Notifications
You must be signed in to change notification settings - Fork 66
Maven artifactIds starting with a number can't rollout from Jenkins #3784
Comments
Would it be better to add checks from launcher backend as well? |
@hrishin agreed. We would be really happy if you would send us a PR ;) |
@gastaldi sent a PR 👍 . Could you please review it? |
@jiekang still it's not working. (Is it in production?)
Can we use this Though it would be very restrictive for application/project names. |
I think the change in fabric8-ui is not in production yet; after it is included, a user should not be able to create a project starting with a number. A better change throughout the whole project is to keep names as unrestricted strings that users can set to whatever, and use an internal 'name' or 'uuid' for the key that is valid for the different APIs we use: Jenkins, Kubernetes, Openshift, etc. |
Though the UI does not allow application names with a number after fabric8-ui/fabric8-ui#3025, this is still an issue as it seems to be using the GitHub repository name.
|
@hrishin why is this assigned to me? The issue is not in fabric8-ui, it's in the build system. |
@hrishin we do not assign people. Assignment == commitment to do the work. So only the person doing the work should assign themselves when they take it on. The only except is the Lead may assign them during planning. |
So after a little more digging with a vert.x http booster GitHub repository that was renamed to start with a number, I have found that it's actually the maven artifactId that triggers this issue. A pipeline with just the GitHub name change passed. After changing the artifactId to match the name, the pipeline failed. For an example see: https://github.com/jiekang/1234-4321/commits/master (This repository may not exist in a week from this comment) With validation in place on f8-ui side, it's no longer possible for users to use the wizard to have a booster repository created that will trigger this, but for custom projects, it may be an issue. Though I don't recall many projects ever having an artifactId starting with a number, I can't find any clear documentation on whether or not it's allowed. From [1], it does seem valid, albeit out of the ordinary. I think it would be best for the Jenkins build system calls to stop using raw strings coming from elsewhere. Maybe it would be okay to apply a lowercase function, and append 'kube-' to all inputs? [1] https://maven.apache.org/guides/mini/guide-naming-conventions.html |
@joshuawilson noted, will keep that in mind 👍 @jiekang thanks a lot for the further investigation on this. 👍 If a user commits artifacts id with an invalid name it would boil down to build system. yeah, let's create the service name by appending |
Added fix for this issue jenkinsci/kubernetes-pipeline-plugin#68 |
…kins For most of the applications, service name is derived from application name. If service name is not adhering to valid DNS characters then applyKubernetes step fails to create k8s resources. To fix this issue, this patch will update(sanitize) the invalid service name by prefixing `svc-`, transforms all characters to small case and updates the service reference in route. This auto-patching can be enabled or disabled by setting ENV var "K8S_PIPELINE_SERVICE_PATCH" to "enabled" or "K8S_PIPELINE_SERVICE_PATCH" to "disabled" (by default its disabled)". Patch has been fixed in kubernetes-pipeline-plugin -> devops project. Changes: - Plugin file built from jenkinsci/kubernetes-pipeline-plugin@e89bc40 - Renamed plugin file from hpi to jpi as jpi has higher precedence - Update version.txt Fixes openshiftio/openshift.io#3784
For most of the applications, service name is derived from application name. If service name is not adhering to valid DNS characters then applyKubernetes step fails to create k8s resources. To fix this issue, this patch will update(sanitize) the invalid service name by prefixing , transforms all characters to small case and updates the service reference in route. This auto-patching can be enabled or disabled by setting ENV var K8S_PIPELINE_SERVICE_PATCH to enabled or K8S_PIPELINE_SERVICE_PATCH to disabled (by default its disabled). Changes: - Added `K8S_PIPELINE_SERVICE_PATCH` ENV VAR in kubernetes and openshift deployments Fixes openshiftio/openshift.io#3784
For most of the applications, service name is derived from application name. If service name is not adhering to valid DNS characters then applyKubernetes step fails to create k8s resources. To fix this issue, this patch updates (sanitizes) the invalid service name by prefixing `svc-`, transforms all characters to small case and updates the service reference in route. This auto-patching can be toggled by setting ENV var `K8S_PIPELINE_SERVICE_PATCH` to `enabled` or to `disabled` (default). Patch has been added in kubernetes-pipeline-plugin -> devops project. Changes: - Plugin has been built from jenkinsci/kubernetes-pipeline-plugin@e89bc40 - Renamed plugin file from hpi to jpi as jpi takes higher precedence - Update version.txt Fixes openshiftio/openshift.io#3784
got closed due to PR merge |
For most of the applications, service name is derived from application name. If service name is not adhering to valid DNS characters then applyKubernetes step fails to create k8s resources. To fix this issue, this patch updates (sanitizes) the invalid service name by prefixing `svc-`, transforms all characters to small case and updates the service reference in route. This auto-patching can be toggled by setting ENV var `K8S_PIPELINE_SERVICE_PATCH` to `enabled` or to `disabled` (default). Changes: - Added `K8S_PIPELINE_SERVICE_PATCH` ENV VAR in kubernetes and openshift deployments Fixes openshiftio/openshift.io#3784
I tried to rollout to stage: Application Name: awtftwtaw and still see failure log:
|
@jiekang the Its not updated in prod yet. |
What's the plan to get it into production? Is there anything blocking it? If it's all good feel free to close this issue again. |
yesterday we were trying to roll out into prod but we are facing issues with builds, hence it is blocked. Will post the update here. |
Just for info: Test build logs https://pastebin.com/3TRm3eUP for the repo https://github.com/hrishin/1222-123236435141. This update is blocked due to #4077 |
Okay, thanks for the updates! |
EDIT:
Scenario 2 steps to reproduce:
More specifically:
The text was updated successfully, but these errors were encountered: