Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-62444

Add and check JCasC compatibility for email-ext plugin

    • Icon: New Feature New Feature
    • Resolution: Fixed
    • Icon: Major Major
    • email-ext-plugin
    • None
    • 2.72

      It would be great if email-ext was compatible with Jenkins Configuration as Code.
      This way the plugin could be configured at Jenkins startup with a single yaml file.

      There is already a bunch of PRs filed on other plugins doing the same, for example how it was done for kubernetes plugin:
      https://github.com/cloudbees/kube-agent-management-plugin/pull/128/files

      Basically the purpose is:

      • Review CasC support, and implement it if not already in place.
      • Review symbols and YAML format so that it's "nice". Example: avoid the unclassified group.
      • Create automated tests to validate the support.

      Thank you.

          [JENKINS-62444] Add and check JCasC compatibility for email-ext plugin

          Alex Earl added a comment -

          I started to look into this at one point and thought it would be good to switch to using the credentials plugin before working on JCasC support. That is still currently my plan.

          Alex Earl added a comment - I started to look into this at one point and thought it would be good to switch to using the credentials plugin before working on JCasC support. That is still currently my plan.

          I'm currently looking at validating the support. I can say that the defaultTriggerIds are not considered.

          The problem seems that the majority of the configuration  is done in ExtendedEmailPublisherDescriptor#configure and not through setters. I'm trying to solve this but I'm afraid I might end up breaking more things than I solve. To be continued.{{}}

          Adrien Lecharpentier added a comment - I'm currently looking at validating the support. I can say that the defaultTriggerIds are not considered. The problem seems that the majority of the configuration  is done in  ExtendedEmailPublisherDescriptor#configure and not through setters. I'm trying to solve this but I'm afraid I might end up breaking more things than I solve. To be continued.{{}}

          Alex Earl added a comment -

          I'm already going down this path, I don't think we should duplicate effort.

          Alex Earl added a comment - I'm already going down this path, I don't think we should duplicate effort.

          Do you know when you can allocate time on this? As you are the maintainer, I'm all in to let you handle this the way you prefer.

          Adrien Lecharpentier added a comment - Do you know when you can allocate time on this? As you are the maintainer, I'm all in to let you handle this the way you prefer.

          I didn't mean to rush you or anything. I just want to see how I can help you best on this.

          Adrien Lecharpentier added a comment - I didn't mean to rush you or anything. I just want to see how I can help you best on this.

          Alex Earl added a comment -

          I'll look at it this next week. I have a repo somewhere with most of the changes.

          Alex Earl added a comment - I'll look at it this next week. I have a repo somewhere with most of the changes.

          Alex, how can I help you with this task? Could you push your branch publicly, maybe we can divide the work to conquer..

          Adrien Lecharpentier added a comment - Alex, how can I help you with this task? Could you push your branch publicly, maybe we can divide the work to conquer..

          Alex Earl added a comment -

          Adrien, Now looking back on this, I remember why it was taking me longer to get it figured out. When using the binding instead of pulling data from the forms manually, there were some things I had to convert to individual jelly files for describable objects. I will push something to a branch soon, I appreciate the help!

          Alex Earl added a comment - Adrien, Now looking back on this, I remember why it was taking me longer to get it figured out. When using the binding instead of pulling data from the forms manually, there were some things I had to convert to individual jelly files for describable objects. I will push something to a branch soon, I appreciate the help!

          Mostly, I deleted the configure method and used @DataboundSetter on the setters. The main issue was on the filters and some other field.

          Adrien Lecharpentier added a comment - Mostly, I deleted the configure method and used @DataboundSetter on the setters. The main issue was on the filters and some other field.

          Alex Earl added a comment -

          Yes, the MailAccount fields and some others. I'll push a branch today.

          Alex Earl added a comment - Yes, the MailAccount fields and some others. I'll push a branch today.

          Alex Earl added a comment -

          I did push a branch: https://github.com/jenkinsci/email-ext-plugin/tree/jcasc_updates, it currently builds and all tests pass, but I would like to extend the tests to do more tests of the global configuration since that is where a lot of changes are.

          Alex Earl added a comment - I did push a branch: https://github.com/jenkinsci/email-ext-plugin/tree/jcasc_updates , it currently builds and all tests pass, but I would like to extend the tests to do more tests of the global configuration since that is where a lot of changes are.

          Adrien Lecharpentier added a comment - - edited

          Thanks Alex. I saw the branch on thursday and it looked good. I'm wondering if we could also have a JCasC specific tests like in https://github.com/jenkinsci/configuration-as-code-plugin/tree/master/integrations/src/test/java/io/jenkins/plugins/casc using a specific yml file to configure the plugin. This could serves as an example.

          I can send you a patch or do a PR against this branch for this. Should be able to do it in the next 24hrs.

          Adrien Lecharpentier added a comment - - edited Thanks Alex. I saw the branch on thursday and it looked good. I'm wondering if we could also have a JCasC specific tests like in https://github.com/jenkinsci/configuration-as-code-plugin/tree/master/integrations/src/test/java/io/jenkins/plugins/casc using a specific yml file to configure the plugin. This could serves as an example. I can send you a patch or do a PR against this branch for this. Should be able to do it in the next 24hrs.

          Alex Earl added a comment -

          Yes, I would definitely like that . I am not super familiar with JCasC, so having someone who is would be great

          Alex Earl added a comment - Yes, I would definitely like that . I am not super familiar with JCasC, so having someone who is would be great

          Hi slide_o_mix, I looked at the test and it seems pretty complete. Do you feel like you need to add something different? Maybe about the triggers, to show how to set them up?

          We could add a new unit test and a resource file with only the triggers for example.

          Adrien Lecharpentier added a comment - Hi slide_o_mix , I looked at the test and it seems pretty complete. Do you feel like you need to add something different? Maybe about the triggers, to show how to set them up? We could add a new unit test and a resource file with only the triggers for example.

          Alex Earl added a comment -

          Yes, I think that would be good.

          Alex Earl added a comment - Yes, I think that would be good.

          Adrien Lecharpentier added a comment - here it is: https://github.com/jenkinsci/email-ext-plugin/pull/211

            slide_o_mix Alex Earl
            mramonleon Ramon Leon
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

              Created:
              Updated:
              Resolved: