jenkinsci / amazon-ecs-plugin

Amazon EC2 Container Service Plugin for Jenkins
https://plugins.jenkins.io/amazon-ecs
MIT License
192 stars 226 forks source link

If Tags are null some null pointer exceptions #290

Closed carpnick closed 1 year ago

carpnick commented 1 year ago

Jenkins and plugins versions report

Environment ```text Jenkins: 2.361.3 OS: Linux - 4.14.294-220.533.amzn2.aarch64 --- ace-editor:1.1 amazon-ecs:1.46 analysis-model-api:10.19.0 ansicolor:1.0.2 ant:481.v7b_09e538fcca antisamy-markup-formatter:155.v795fb_8702324 apache-httpcomponents-client-4-api:4.5.13-138.v4e7d9a_7b_a_e61 artifactory:3.17.3 authentication-tokens:1.4 aws-credentials:191.vcb_f183ce58b_9 aws-java-sdk:1.12.287-357.vf82d85a_6eefd aws-java-sdk-cloudformation:1.12.287-357.vf82d85a_6eefd aws-java-sdk-codebuild:1.12.287-357.vf82d85a_6eefd aws-java-sdk-ec2:1.12.287-357.vf82d85a_6eefd aws-java-sdk-ecr:1.12.287-357.vf82d85a_6eefd aws-java-sdk-ecs:1.12.287-357.vf82d85a_6eefd aws-java-sdk-efs:1.12.287-357.vf82d85a_6eefd aws-java-sdk-elasticbeanstalk:1.12.287-357.vf82d85a_6eefd aws-java-sdk-iam:1.12.287-357.vf82d85a_6eefd aws-java-sdk-logs:1.12.287-357.vf82d85a_6eefd aws-java-sdk-minimal:1.12.287-357.vf82d85a_6eefd aws-java-sdk-sns:1.12.287-357.vf82d85a_6eefd aws-java-sdk-sqs:1.12.287-357.vf82d85a_6eefd aws-java-sdk-ssm:1.12.287-357.vf82d85a_6eefd basic-branch-build-strategies:71.vc1421f89888e blueocean-commons:1.25.8 blueocean-jira:1.25.8 blueocean-rest:1.25.8 bootstrap5-api:5.2.1-3 bouncycastle-api:2.26 branch-api:2.1051.v9985666b_f6cc build-timeout:1.24 caffeine-api:2.9.3-65.v6a_47d0f4d1fe checks-api:1.8.0 cloudbees-bitbucket-branch-source:791.vb_eea_a_476405b cloudbees-folder:6.758.vfd75d09eea_a_1 codebuild-cloud:0.9.31.v3c6129fc7329 command-launcher:90.v669d7ccb_7c31 commons-lang3-api:3.12.0-36.vd97de6465d5b_ commons-text-api:1.10.0-27.vb_fa_3896786a_7 config-file-provider:3.11.1 configuration-as-code:1569.vb_72405b_80249 configuration-as-code-groovy:1.1 credentials:1189.vf61b_a_5e2f62e credentials-binding:523.vd859a_4b_122e6 dark-theme:262.v0202a_4c8fb_6a data-tables-api:1.12.1-4 display-url-api:2.3.6 durable-task:501.ve5d4fc08b0be echarts-api:5.4.0-1 email-ext:2.92 font-awesome-api:6.2.0-3 forensics-api:1.16.0 git:4.13.0 git-client:3.13.0 github:1.36.0 github-api:1.303-400.v35c2d8258028 github-branch-source:1696.v3a_7603564d04 gradle:2.1 handlebars:3.0.8 handy-uri-templates-2-api:2.1.8-22.v77d5b_75e6953 htmlpublisher:1.31 http_request:1.16 ignore-committer-strategy:1.0.4 instance-identity:116.vf8f487400980 ionicons-api:31.v4757b_6987003 ivy:2.2 jackson2-api:2.13.4.20221013-295.v8e29ea_354141 jakarta-activation-api:2.0.1-2 jakarta-mail-api:2.0.1-2 javadoc:226.v71211feb_e7e9 javax-activation-api:1.2.0-5 javax-mail-api:1.6.2-8 jaxb:2.3.7-1 jdk-tool:63.v62d2fd4b_4793 jersey2-api:2.37-1 jira:3.8 jjwt-api:0.11.5-77.v646c772fddb_0 jquery3-api:3.6.1-2 jsch:0.1.55.61.va_e9ee26616e7 junit:1156.vcf492e95a_a_b_0 ldap:2.12 lockable-resources:2.18 mailer:438.v02c7f0a_12fa_4 mask-passwords:3.3 matrix-auth:3.1.5 matrix-project:785.v06b_7f47b_c631 maven-plugin:3.20 mina-sshd-api-common:2.9.1-44.v476733c11f82 mina-sshd-api-core:2.9.1-44.v476733c11f82 momentjs:1.1.1 monitoring:1.91.0 okhttp-api:4.9.3-108.v0feda04578cf pam-auth:1.10 permissive-script-security:0.7 pipeline-aws:1.43 pipeline-build-step:2.18 pipeline-github-lib:38.v445716ea_edda_ pipeline-graph-analysis:195.v5812d95a_a_2f9 pipeline-groovy-lib:613.v9c41a_160233f pipeline-input-step:456.vd8a_957db_5b_e9 pipeline-milestone-step:101.vd572fef9d926 pipeline-model-api:2.2118.v31fd5b_9944b_5 pipeline-model-definition:2.2118.v31fd5b_9944b_5 pipeline-model-extensions:2.2118.v31fd5b_9944b_5 pipeline-rest-api:2.27 pipeline-stage-step:296.v5f6908f017a_5 pipeline-stage-tags-metadata:2.2118.v31fd5b_9944b_5 pipeline-stage-view:2.27 plain-credentials:139.ved2b_9cf7587b plugin-util-api:2.18.0 popper2-api:2.11.6-2 prism-api:1.29.0-1 pyenv-pipeline:2.1.2 rebuild:1.34 resource-disposer:0.20 saml:4.372.v89f13e4c9e97 scm-api:621.vda_a_b_055e58f7 script-security:1189.vb_a_b_7c8fd5fde skip-notifications-trait:82.vc7780f4b_0373 snakeyaml-api:1.33-90.v80dcb_3814d35 ssh-credentials:305.v8f4381501156 ssh-slaves:2.854.v7fd446b_337c9 sshd:3.249.v2dc2ea_416e33 structs:324.va_f5d6774f3a_d theme-manager:1.5 timestamper:1.21 token-macro:308.v4f2b_ed62b_b_16 trilead-api:2.72.v2a_3236754f73 uno-choice:2.6.4 variant:59.vf075fe829ccb warnings-ng:9.20.1 workflow-aggregator:590.v6a_d052e5a_a_b_5 workflow-api:1200.v8005c684b_a_c6 workflow-basic-steps:994.vd57e3ca_46d24 workflow-cps:3536.vb_8a_6628079d5 workflow-durable-task-step:1210.va_1e5d77e122b workflow-job:1254.v3f64639b_11dd workflow-multibranch:716.vc692a_e52371b_ workflow-scm-step:400.v6b_89a_1317c9a_ workflow-step-api:639.v6eca_cd8c04a_a_ workflow-support:839.v35e2736cfd5c ws-cleanup:0.43 ```

What Operating System are you using (both controller, and any agents involved in the problem)?

Containers

Reproduction steps

If you setup your jenkins with Code (Groovy), and you pass null into Tags it generates a null reference exception.

Presumedly the UI is creating an empty list.

Expected Results

No null pointer exceptions

Actual Results

image

Anything else?

Workaround: Set Tags in constructor to an empty list. ([])

ugurkany commented 1 year ago

@carpnick Thank you for your feedback! I will fix this as soon as possible and release a new version.

carpnick commented 1 year ago

Might be worth noting the extra permissions required too: I know you need this one: "ecs:ListTagsForResource"

Not sure if you need others.

carpnick commented 1 year ago

Also not sure if you want me to open another bug or not.... But Match on Tags is generating too many task definitions:

image

You can see here when I specify empty tag set - it never matches. I am guessing this is because of these lines. Those lines are automatically getting added on template generation. But then here you are comparing the template from ECS to the plugin config. The plugin config doesnt automatically add the default tags on template generation that the plugin is adding, so this will never match as designed.

I would suggest comparing tags after you remove the 2 default ones.

So if you want something like this: templateTagsMatchesExistingTags = ObjectUtils.equals(template.getTags(), tags);

Write it like this:

List<Tag> comparisonTags = new ArrayList<Tag>();
List<Tag> realTemplateTags = template.getTags()
for(Tag item : realTemplateTags){
    if(item. name.equals(AWS_TAG_JENKINS_LABEL_KEY){
       continue;
     }
     else if(item. name.equals(AWS_TAG_JENKINS_TEMPLATENAME_KEY){
        continue;
     }
     else{ 
         comparisonTags.add(item)
     }
}

//Finally
 templateTagsMatchesExistingTags = ObjectUtils.equals(comparisonTags, tags);