Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jenkinsfile is not overwritten #52

Closed
gounthar opened this issue Nov 10, 2023 · 5 comments · Fixed by openrewrite/rewrite#3682
Closed

Jenkinsfile is not overwritten #52

gounthar opened this issue Nov 10, 2023 · 5 comments · Fixed by openrewrite/rewrite#3682
Assignees
Labels
bug Something isn't working

Comments

@gounthar
Copy link
Contributor

What version of OpenRewrite are you using?

I'm using it through Moderne.io, so I don't know, sorry.

How are you running OpenRewrite?

I'm using Moderne.io.
I don't think you will be able to see the results, but I used my latest recipe of recipes that gave: https://app.moderne.io/results/Dm3ZuYTUW

The plugin I tried this on is multibranch-scan-webhook-trigger-plugin.
You can see the first commit there: jenkinsci/multibranch-scan-webhook-trigger-plugin@fde4557

You can replay the recipe of recipes here: https://app.moderne.io/recipes/builder/j0daJWhoh?organizationId=SmVua2lucyBDSQ%3D%3D .

What is the smallest, simplest way to reproduce the problem?

Re-run the recipe.
A friend of mine tried only the modernize Jenkinsfile on his machine with the same project thanks to the maven command, and it failed to change the file too.

mvn -U org.openrewrite.maven:rewrite-maven-plugin:run \
  -Drewrite.recipeArtifactCoordinates=org.openrewrite.recipe:rewrite-jenkins:RELEASE \
  -Drewrite.activeRecipes=org.openrewrite.jenkins.ModernizeJenkinsfile

Once the file was deleted, the recipe created the file correctly.

What did you expect to see?

Well, at least a rewritten Jenkinsfile, even if the complexity of the Jenkinsfile was lost (some of them are not just buildplugin(), but lots of instructions like in https://raw.githubusercontent.com/jenkinsci/kubernetes-plugin/master/Jenkinsfile.

What did you see instead?

Nothing, until we deleted the local Jenkinsfile.

What is the full stack trace of any errors you encountered?

No error.

Are you interested in [contributing a fix to OpenRewrite]

I'd love to contribute if possible. While I may not have all the required skills, I'm enthusiastic about the project and willing to learn.

@gounthar gounthar added the bug Something isn't working label Nov 10, 2023
@timtebeek
Copy link
Contributor

Thanks for reporting your issue here and offer to help! I've had a quick look, adding some diagnostics from the serialized LST manifest.csv:

n,sourcePath,sourceFileType,buildTool,buildToolVersion,checksum
1,pom.xml,Xml.Document,moderne-maven-plugin,2.4.3,35afca4aa95d5230b61a9dac0ee7e038
2,src/main/java/com/igalg/jenkins/plugins/mswt/ComputedFolderWebHookRequestReceiver.java,J.CompilationUnit,moderne-maven-plugin,2.4.3,f8fab40063f5e38bc384455dc8958de4
3,src/main/java/com/igalg/jenkins/plugins/mswt/trigger/ComputedFolderWebHookTriggerResult.java,J.CompilationUnit,moderne-maven-plugin,2.4.3,6919234fdcf6c0f02d37de48e60fa741
4,src/main/java/com/igalg/jenkins/plugins/mswt/trigger/ComputedFolderWebHookTrigger.java,J.CompilationUnit,moderne-maven-plugin,2.4.3,c8f276ddacf0359ab40224cb7fd23307
5,src/main/java/com/igalg/jenkins/plugins/mswt/locator/JenkinsInstanceComputedFolderLocator.java,J.CompilationUnit,moderne-maven-plugin,2.4.3,b8f628482d4f9af09de8f95427cb3333
6,src/main/java/com/igalg/jenkins/plugins/mswt/locator/ComputedFolderLocator.java,J.CompilationUnit,moderne-maven-plugin,2.4.3,1c544f45f9300552b9bc7c7172a49875
7,src/main/java/com/igalg/jenkins/plugins/mswt/locator/LocatedComputedFolder.java,J.CompilationUnit,moderne-maven-plugin,2.4.3,a6d1674f09e2c3ad3c7faf5d9f79422f
8,src/main/java/com/igalg/jenkins/plugins/mswt/locator/ComputedFolderWebHookTriggerLocator.java,J.CompilationUnit,moderne-maven-plugin,2.4.3,685d718498d611c44a00b27042a9cd64
9,src/main/resources/index.jelly,Quark,moderne-maven-plugin,2.4.3,d08d63152a65ac77e6f43ec6fb67c194
10,src/main/resources/com/igalg/jenkins/plugins/mswt/trigger/ComputedFolderWebHookTrigger/help-token.html,Quark,moderne-maven-plugin,2.4.3,4ffff340bf411198149254dfd164b4c6
11,src/main/resources/com/igalg/jenkins/plugins/mswt/trigger/ComputedFolderWebHookTrigger/config.jelly,Quark,moderne-maven-plugin,2.4.3,52e4259cdd96d3eeef3697ab71037e67
12,src/main/resources/com/igalg/jenkins/plugins/mswt/trigger/ComputedFolderWebHookTrigger/help.html,Quark,moderne-maven-plugin,2.4.3,fff04db5168fb000d58fca3bf29fc422
13,src/test/java/com/igalg/jenkins/plugins/mswt/ComputedFolderWebHookRequestReceiverTest.java,J.CompilationUnit,moderne-maven-plugin,2.4.3,27e1e81245baa61c1403b9ac0cb589d7
14,src/test/java/com/igalg/jenkins/plugins/mswt/locator/ComputedFolderWebHookTriggerLocatorTest.java,J.CompilationUnit,moderne-maven-plugin,2.4.3,2cb32359672c6b6e05005881d0cef615
15,Jenkinsfile,G.CompilationUnit,mod-java,v0.1.1-SNAPSHOT,82c77d7c20e52b57a2b5a886e4a22803
16,.gitignore,PlainText,native,v1.6.0,3b37ecfb3c733c8bcd5bc46c842fcd58
17,LICENSE,Quark,native,v1.6.0,a8c90949a803f50e7f2359fc05a0fa9d
18,README.md,PlainText,native,v1.6.0,13978609ca77aa53a2298b5b32f85fff
19,images/configure-token.png,Quark,native,v1.6.0,3b577d022f94f9cab92a069022cce4ea

I don't yet know what's going on; especially since you're reporting the same issue locally. 🤔

I had thought there might be a parse issue, but from the above that does not immediately seem the case.

@timtebeek
Copy link
Contributor

Gave it a go here as well just now; seeing the same behaviour, and can trace that back to how the recipe is implemented:

name: org.openrewrite.jenkins.ModernizeJenkinsfile
displayName: Modernize Jenkinsfile
description: Updates `Jenkinsfile` to build with recommended Java versions, platforms, and settings.
recipeList:
- org.openrewrite.text.CreateTextFile:
relativeFileName: Jenkinsfile
overwriteExisting: true
fileContents: >
/*
See the documentation for more options:
https://github.com/jenkins-infra/pipeline-library/
*/
buildPlugin(
useContainerAgent: true, // Set to `false` if you need to use Docker for containerized tests
configurations: [
[platform: 'linux', jdk: 21],
[platform: 'windows', jdk: 17],
])

The recipe right now does not override files of a different type, as evidenced by this failing test:

    @Test
    void shouldOverrideExisting() {
        String after = """
          /*
           See the documentation for more options:
           https://github.com/jenkins-infra/pipeline-library/
          */
          buildPlugin(
            useContainerAgent: true, // Set to `false` if you need to use Docker for containerized tests
            configurations: [
              [platform: 'linux', jdk: 21],
              [platform: 'windows', jdk: 17],
          ])
          """;
        rewriteRun(
          spec -> spec.recipe(new CreateTextFile(after, "Jenkinsfile", true)),
          groovy(
            """
              #!groovy
              buildPlugin()
              """,
            after,
            spec -> spec.path("Jenkinsfile")
          )
        );
    }

I'll see if I can fix that in OpenRewrite/rewrite itself

@timtebeek
Copy link
Contributor

Picked up in

The problem is a side effect of now parsing Jenkinsfiles as Groovy rather than PlainText that we did before. Thanks for bringing this to our attention!

@timtebeek timtebeek self-assigned this Nov 12, 2023
@timtebeek timtebeek moved this to In Progress in OpenRewrite Nov 12, 2023
@timtebeek timtebeek moved this from In Progress to Ready to Review in OpenRewrite Nov 12, 2023
@github-project-automation github-project-automation bot moved this from Ready to Review to Done in OpenRewrite Nov 13, 2023
@timtebeek
Copy link
Contributor

The underlying issue has been fixed, and should roll out to our snapshot versions soon. We deploy the latest changes to app.moderne.io such that it should be usable for the next batch of projects again.

I see you've already made the required changes to your jdk-21-prerequisites-and-powermock branch, such that I think we don't have to expedite a deploy & release, right?

Thanks again for pointing out this issue!

@gounthar
Copy link
Contributor Author

That's right, @timtebeek , but I will try it on other repositories.
Thanks a bunch for the super fast fix. 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants