jenkinsci / extension-filter-plugin

https://plugins.jenkins.io/extension-filter/
MIT License
2 stars 7 forks source link

Latest release broke my use of this plugin #65

Closed iciclespider closed 8 months ago

iciclespider commented 8 months ago

Jenkins and plugins versions report

Environment ```text Jenkins: 2.447 OS: Linux - 5.10.209-198.812.amzn2.x86_64 Java: 17.0.10 - Eclipse Adoptium (OpenJDK 64-Bit Server VM) --- Office-365-Connector:4.21.0 amazon-ecr:1.114.vfd22430621f5 ansicolor:1.0.4 antisamy-markup-formatter:162.v0e6ec0fcfcf6 apache-httpcomponents-client-4-api:4.5.14-208.v438351942757 authentication-tokens:1.53.v1c90fd9191a_b_ authorize-project:1.7.1 aws-credentials:218.v1b_e9466ec5da_ aws-java-sdk:1.12.633-430.vf9a_e567a_244f aws-java-sdk-cloudformation:1.12.633-430.vf9a_e567a_244f aws-java-sdk-codebuild:1.12.633-430.vf9a_e567a_244f aws-java-sdk-ec2:1.12.633-430.vf9a_e567a_244f aws-java-sdk-ecr:1.12.633-430.vf9a_e567a_244f aws-java-sdk-ecs:1.12.633-430.vf9a_e567a_244f aws-java-sdk-efs:1.12.633-430.vf9a_e567a_244f aws-java-sdk-elasticbeanstalk:1.12.633-430.vf9a_e567a_244f aws-java-sdk-iam:1.12.633-430.vf9a_e567a_244f aws-java-sdk-kinesis:1.12.633-430.vf9a_e567a_244f aws-java-sdk-logs:1.12.633-430.vf9a_e567a_244f aws-java-sdk-minimal:1.12.633-430.vf9a_e567a_244f aws-java-sdk-secretsmanager:1.12.633-430.vf9a_e567a_244f aws-java-sdk-sns:1.12.633-430.vf9a_e567a_244f aws-java-sdk-sqs:1.12.633-430.vf9a_e567a_244f aws-java-sdk-ssm:1.12.633-430.vf9a_e567a_244f azure-ad:457.vf85d61f83b_26 azure-sdk:157.v855da_0b_eb_dc2 basic-branch-build-strategies:81.v05e333931c7d blueocean:1.27.11 blueocean-bitbucket-pipeline:1.27.11 blueocean-commons:1.27.11 blueocean-config:1.27.11 blueocean-core-js:1.27.11 blueocean-dashboard:1.27.11 blueocean-display-url:2.4.2 blueocean-events:1.27.11 blueocean-git-pipeline:1.27.11 blueocean-github-pipeline:1.27.11 blueocean-i18n:1.27.11 blueocean-jwt:1.27.11 blueocean-personalization:1.27.11 blueocean-pipeline-api-impl:1.27.11 blueocean-pipeline-editor:1.27.11 blueocean-pipeline-scm-api:1.27.11 blueocean-rest:1.27.11 blueocean-rest-impl:1.27.11 blueocean-web:1.27.11 bootstrap5-api:5.3.2-4 bouncycastle-api:2.30.1.77-225.v26ea_c9455fd9 branch-api:2.1152.v6f101e97dd77 caffeine-api:3.1.8-133.v17b_1ff2e0599 checks-api:2.0.2 cloudbees-bitbucket-branch-source:877.vb_b_d5243f6794 cloudbees-folder:6.928.v7c780211d66e cloudops:1.0.0 command-launcher:107.v773860566e2e commons-lang3-api:3.13.0-62.v7d18e55f51e2 commons-text-api:1.11.0-95.v22a_d30ee5d36 credentials:1319.v7eb_51b_3a_c97b_ credentials-binding:657.v2b_19db_7d6e6d dashboard-view:2.508.va_74654f026d1 disk-usage:1.2 display-url-api:2.200.vb_9327d658781 docker-commons:439.va_3cb_0a_6a_fb_29 downstream-build-cache:1.7 durable-task:550.v0930093c4b_a_6 ec2:1648.vf3d852e00486 echarts-api:5.4.3-4 envinject:2.908.v66a_774b_31d93 envinject-api:1.199.v3ce31253ed13 extension-filter:126.v6537d3078db_d favorite:2.208.v91d65b_7792a_c folder-properties:1.2.1 font-awesome-api:6.5.1-3 git:5.2.1 git-client:4.6.0 github:1.38.0 github-api:1.318-461.v7a_c09c9fa_d63 github-branch-source:1772.va_69eda_d018d4 gitlab-api:5.3.0-91.v1f9a_fda_d654f gitlab-branch-source:702.v7dde70ed1522 gson-api:2.10.1-15.v0d99f670e0a_7 handy-uri-templates-2-api:2.1.8-30.v7e777411b_148 htmlpublisher:1.32 instance-identity:185.v303dc7c645f9 ionicons-api:56.v1b_1c8c49374e jackson2-api:2.16.1-373.ve709c6871598 jacoco:3.3.5 jakarta-activation-api:2.0.1-3 jakarta-mail-api:2.0.1-3 javax-activation-api:1.2.0-6 javax-mail-api:1.6.2-9 jaxb:2.3.9-1 jdk-tool:73.vddf737284550 jenkins-design-language:1.27.11 jersey2-api:2.41-133.va_03323b_a_1396 jjwt-api:0.11.5-77.v646c772fddb_0 job-dsl:1.87 joda-time-api:2.12.7-29.v5a_b_e3a_82269a_ jquery3-api:3.7.1-2 json-path-api:2.9.0-33.v2527142f2e1d junit:1259.v65ffcef24a_88 junit-attachments:205.vc0677977deb_0 kubernetes:4186.v1d804571d5d4 kubernetes-client-api:6.10.0-240.v57880ce8b_0b_2 kubernetes-credentials:0.11 mailer:463.vedf8358e006b_ matrix-auth:3.2.1 matrix-project:822.824.v14451b_c0fd42 metrics:4.2.21-449.v6960d7c54c69 mina-sshd-api-common:2.12.0-90.v9f7fb_9fa_3d3b_ mina-sshd-api-core:2.12.0-90.v9f7fb_9fa_3d3b_ monitoring:1.98.0 multibranch-build-strategy-extension:48.v3dc306525d0c node-iterator-api:55.v3b_77d4032326 okhttp-api:4.11.0-172.vda_da_1feeb_c6e pipeline-aws:1.43 pipeline-build-step:540.vb_e8849e1a_b_d8 pipeline-graph-analysis:216.vfd8b_ece330ca_ pipeline-groovy-lib:704.vc58b_8890a_384 pipeline-input-step:491.vb_07d21da_1a_fb_ pipeline-milestone-step:111.v449306f708b_7 pipeline-model-api:2.2175.v76a_fff0a_2618 pipeline-model-definition:2.2175.v76a_fff0a_2618 pipeline-model-extensions:2.2175.v76a_fff0a_2618 pipeline-rest-api:2.34 pipeline-stage-step:305.ve96d0205c1c6 pipeline-stage-tags-metadata:2.2175.v76a_fff0a_2618 pipeline-stage-view:2.34 pipeline-utility-steps:2.16.2 plain-credentials:143.v1b_df8b_d3b_e48 plugin-util-api:4.1.0 pubsub-light:1.18 scm-api:683.vb_16722fb_b_80b_ scm-filter-branch-pr:148.v0b_5f06e8b_c84 script-security:1326.vdb_c154de8669 snakeyaml-api:2.2-111.vc6598e30cc65 sse-gateway:1.26 ssh-agent:346.vda_a_c4f2c8e50 ssh-credentials:308.ve4497b_ccd8f4 sshd:3.312.v1c601b_c83b_0e structs:337.v1b_04ea_4df7c8 test-results-analyzer:0.4.1 timestamper:1.26 token-macro:400.v35420b_922dcb_ trilead-api:2.133.vfb_8a_7b_9c5dd1 uno-choice:2.8.1 variant:60.v7290fc0eb_b_cd workflow-aggregator:596.v8c21c963d92d workflow-api:1291.v51fd2a_625da_7 workflow-basic-steps:1042.ve7b_140c4a_e0c workflow-cps:3880.vb_ef4b_5cfd270 workflow-durable-task-step:1331.vc8c2fed35334 workflow-job:1400.v7fd111b_ec82f workflow-multibranch:783.va_6eb_ef636fb_d workflow-scm-step:415.v434365564324 workflow-step-api:657.v03b_e8115821b_ workflow-support:865.v43e78cc44e0d yet-another-build-visualizer:1.16 ```

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

Controller: debian:bookworm

Reproduction steps

With the Kubernetes plugin installed configure the Extension Filter with the following Exclusion:

Extension class
org.csanchez.jenkins.plugins.kubernetes.pod.retention.Reaper$TerminateAgentOnImagePullBackOff

Context
org.csanchez.jenkins.plugins.kubernetes.pod.retention.Reaper$Listener

Note that this does display "Error. Class not found" under the Extension class, but that happens in both the working and non working versions.

Next, run the following in the Script Console:

import org.csanchez.jenkins.plugins.kubernetes.pod.retention.Reaper
Jenkins.get().getExtensionList(Reaper.Listener).each { println it }
return

Expected Results

In version 102.v3c515b_7a9efb of this plugin, the output is:

org.csanchez.jenkins.plugins.kubernetes.pod.retention.Reaper$RemoveAgentOnPodDeleted@51a97a4e
org.csanchez.jenkins.plugins.kubernetes.pod.retention.Reaper$TerminateAgentOnContainerTerminated@1533daaa
org.csanchez.jenkins.plugins.kubernetes.pod.retention.Reaper$TerminateAgentOnPodFailed@6128123c

Actual Results

In version 126.v6537d3078db_dof this plugin, the output is:

org.csanchez.jenkins.plugins.kubernetes.pod.retention.Reaper$RemoveAgentOnPodDeleted@53f66bfe
org.csanchez.jenkins.plugins.kubernetes.pod.retention.Reaper$TerminateAgentOnContainerTerminated@2720519e
org.csanchez.jenkins.plugins.kubernetes.pod.retention.Reaper$TerminateAgentOnImagePullBackOff@4c647eba
org.csanchez.jenkins.plugins.kubernetes.pod.retention.Reaper$TerminateAgentOnPodFailed@19921aaf

Anything else?

I suspect the use of inner classes might be the issue.

Are you interested in contributing a fix?

No response

jonesbusy commented 8 months ago

Hi,

Does this happen only with this specific descriptor ?

I didn't experienced the same for other classes (Like freestyle jobs or tools).

I need to try to reproduce it

FYI @sridamul

sridamul commented 8 months ago

Note that this does display "Error. Class not found" under the Extension class, but that happens in both the working and non working versions.

Hey @jonesbusy does the extension should be removed even if the class not found?

jonesbusy commented 8 months ago

@sridamul How can you remove something that doesn't exist ?

The class not found existed before, and not sure why it happen

Now it seems that some extension cannot be disabled anymore.

Could you test with the Kubernetes plugin and this particular extension if it can be disabled ?

sridamul commented 8 months ago

I tried testing with the kubernetes plugin

Screenshot 2024-03-02 113522

As he said, the class not found arises, but still the class get's removed in the older version

sridamul commented 8 months ago

Extension class org.csanchez.jenkins.plugins.kubernetes.pod.retention.Reaper$TerminateAgentOnImagePullBackOff

But this class is marked as an extension https://github.com/jenkinsci/kubernetes-plugin/blob/1d804571d5d4d2d0bbfe6a16016e128fe4b0da40/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/retention/Reaper.java#L534

jonesbusy commented 8 months ago

Does this mean that the groovy console or other mechanism bypass the filter ?

sridamul commented 8 months ago
            public FormValidation doCheckFqcn(@QueryParameter String fqcn) {
            Class definedClass;
            try {
                definedClass = Class.forName(fqcn);
            } catch (ClassNotFoundException e) {
                return FormValidation.error(Messages.Exclusion_classNotFoundMsg());
            }

            final boolean isExtension = ExtensionPoint.class.isAssignableFrom(definedClass);
            final boolean isDescriptor = Descriptor.class.isAssignableFrom(definedClass);

            // Handle possible combinations
            if (isExtension && isDescriptor) {
                return FormValidation.ok(Messages.Exclusion_classIsExtensionAndDescriptor());
            }
            if (isExtension) {
                return FormValidation.ok(Messages.Exclusion_classIsExtension());
            }
            if (isDescriptor) {
                return FormValidation.ok(Messages.Exclusion_classIsDescriptor());
            }
            return FormValidation.error(Messages.Exclusion_unsupportedType());
        }

My doubt is why the extension fails to be defined as a class?

sridamul commented 8 months ago

Even

org.csanchez.jenkins.plugins.kubernetes.pod.retention.Reaper$TerminateAgentOnContainerTerminated

is marked as an extension https://github.com/jenkinsci/kubernetes-plugin/blob/1d804571d5d4d2d0bbfe6a16016e128fe4b0da40/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pod/retention/Reaper.java#L425

But this isn't removed when I add this extension to the extension-filter-plugin in the older version

iciclespider commented 8 months ago

This is my only use of this plugin, so I do not know if it is working, or not working, with any other Extension classes.

As I mentioned in the Anything else? section, I strongly suspect this has to do with the Java inner usage. I also suspect that erroneous "Error: Class not found" message might be at the heart of the change in behavior. Is it possible that the refactor nows runs that validation process during the use of the plugin? IOW, before it properly did the filtering. Now, it first runs that validation process before deciding to apply the filtering.

jonesbusy commented 8 months ago

In any case I think the validation need simply to be removed.

Due to classloading https://www.jenkins.io/doc/developer/plugin-development/dependencies-and-class-loading/ we cannot access class that this plugin doesn't depends on. So Class.forName will always fail

For the REST my feeling is that hidden descriptor works but broke extension filtering.

I think the issue is 'EXTENSION_FILTER' is never used anymore

I think it must be rollback to

    @Extension
    public static final ExtensionFilter EXTENSION_FILTER = new ExtensionFilter() {
        @Override
        public <T> boolean allows(Class<T> tClass, ExtensionComponent<T> tExtensionComponent) {
            // Don't exclude myself
            return tExtensionComponent.getInstance() != this
                    && DESCRIPTOR.allows(tClass.getName())
                    && DESCRIPTOR.allows(
                            tExtensionComponent.getInstance().getClass().getName());
        }
    };

And fixing the Guice circular dependency an other way (maybe extra this a class and pass the 'DESCRIPTOR'

jonesbusy commented 8 months ago

@iciclespider Thanks for reporting, in the meanwhile can you rollback to previous version ?

sridamul commented 8 months ago

Hey @jonesbusy Then the older version should remove any extensions right? But the older version ~fails to remove~ successfully removes the following too:

org.csanchez.jenkins.plugins.kubernetes.pod.retention.Reaper$TerminateAgentOnContainerTerminated org.csanchez.jenkins.plugins.kubernetes.pod.retention.Reaper$TerminateAgentOnPodFailed

Sorry, wrongly tested with the different version

jonesbusy commented 8 months ago

The @Extension is missing on the new version and is never called. Test are not checking this. Ideally we should have some integration test to validate that extension are filtered correctly.

Can you partially rollback the ExtensionFilterso that I can perform a release during the day.

I don't have time to try to investigate more or find a better solution to fix the original Guice issue.

Thanks

sridamul commented 8 months ago

The @Extension is missing on the new version and is never called. Test are not checking this. Ideally we should have some integration test to validate that extension are filtered correctly.

Oops! sorry for not testing properly

jonesbusy commented 8 months ago

No problem it happen. Testing included here https://github.com/jenkinsci/extension-filter-plugin/pull/64 was about Descriptor which use a different extension point

sridamul commented 8 months ago

Testing included here https://github.com/jenkinsci/extension-filter-plugin/pull/64 was about Descriptor which use a different extension point

just realised now, messed up a lot. I will try to resolve the guice issue with a better approach along with testing the extensions as well

jonesbusy commented 8 months ago

Should be fixed by https://github.com/jenkinsci/extension-filter-plugin/releases/tag/128.v9d95f54d3405

jonesbusy commented 8 months ago

@sridamul

I think creating a class (not inner / anonymous) fixes the issue with Guice

@Extension
public class ConfigurableExtension extends ExtensionFilter {

    public static final Logger LOGGER = Logger.getLogger(ConfigurableExtensionFilter.class.getName());

    @Override
    public <T> boolean allows(Class<T> tClass, ExtensionComponent<T> tExtensionComponent) {

        ConfigurableExtensionFilter.DescriptorImpl descriptor = ConfigurableExtensionFilter.DESCRIPTOR;

        if (descriptor == null) {
            return true;
        }

        // Don't exclude myself
        if (tExtensionComponent.getInstance() == this) return true;
        LOGGER.fine("Checking if " + tClass.getName() + " is allowed as an extension point.");
        boolean allowed = descriptor.allows(tClass.getName())
                && descriptor.allows(
                        tExtensionComponent.getInstance().getClass().getName());
        if (allowed) {
            LOGGER.fine("Extension is allowed: " + tClass.getName());
        } else {
            LOGGER.fine("Extension is blocked: " + tClass.getName());
        }
        return allowed;
    }
}
iciclespider commented 8 months ago

@iciclespider Thanks for reporting, in the meanwhile can you rollback to previous version ?

Yes, I have already done that, I am good for now.

iciclespider commented 8 months ago

I just installed the new release and that looks much better. Still gets the "Error. Class not found" error though, but the filtering is now working again.

jonesbusy commented 8 months ago

The point of validation is something that never worked I think.

This is fixed by https://github.com/jenkinsci/extension-filter-plugin/pull/70 by using the Uberclass loader from plugin manager that as the visibility on all plugin.

Class.forName will look only from what's visible of the plugin

https://www.jenkins.io/doc/developer/plugin-development/dependencies-and-class-loading/

iciclespider commented 8 months ago

And now no more "Error: Class not found"!!! Thanks for your prompt resolution.