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

[BUG] Files in the target folder are being analyzed for builtin provider rules #358

Open
1 task done
jmle opened this issue Oct 5, 2023 · 7 comments
Open
1 task done
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Milestone

Comments

@jmle
Copy link
Contributor

jmle commented Oct 5, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Konveyor version

0.3.0-alpha.5

Priority

Major

Current Behavior

Duplicate entries are appearing because some files inside the target folder are being analyzed.

Expected Behavior

When analyzing a source code project with "source only", the target folder appearing as a result of running maven compilation should not be included in the analysis.

How Reproducible

Always (Default)

Steps To Reproduce

  1. Given the environment, analyze the application using "Source + dependencies".
  2. Look for the issue "Replace the Java EE persistence namespace, schemaLocation and version with the Jakarta equivalent", with rule ID javaee-to-jakarta-namespaces-00002.
  3. Click on "View affected files" and see how two files within a target folder are included:
    image

Environment

* Fedora 38
* minikube version: v1.31.2
* Konveyor 0.3.0-alpha.5
* [sample.daytrader7 app (master branch)](https://github.com/WASdev/sample.daytrader7)
* Analysis configuration:
** Source code + dependencies
** eap8, quarkus, openjdk17, azure, cloud-readiness, camel3
** Application and internal dependencies only
** No custom rules
** No additional advanced options

Anything else?

No response

@jmle jmle added kind/bug Categorizes issue or PR as related to a bug. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Oct 5, 2023
@shawn-hurley
Copy link
Contributor

I don't know if I would consider this a bug, we are finding real things in the project layout.

We could give users some way to filter out specific directories using the built-in provider, but as this does not exist, I worry that this is another feature that needs to be added.

@jmle
Copy link
Contributor Author

jmle commented Oct 6, 2023

@shawn-hurley this is mainly producing duplicate incidents, so I think it's not a huge problem. But at some point we'll probably have to fix it.

@pranavgaikwad pranavgaikwad removed the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Oct 26, 2023
@jwmatthews
Copy link
Member

I hit this with https://github.com/deewhyweb/eap-coolstore-monolith/tree/main

What surprised me is I didn't intentionally compile the application, so was confused when I saw references to a 'target' directory. I now assume that during analysis the target directory was created and hence analyzed.

The real thing that threw me off with this is I was programmatically grabbing the files from Git that the analyzer flagged as an issue, and when I hit the 'target/*' files they were not in Git so I threw an exception in my code. For now I'll just work around this


javaee-to-jakarta-namespaces-00002:
      description: |-
        Replace the Java EE persistence namespace, schemaLocation and version with the Jakarta equivalent
        Replace `http://xmlns.jcp.org/xml/ns/persistence` with `https://jakarta.ee/xml/ns/persistence` and change the schema version number
      category: mandatory
      labels:
      - konveyor.io/target=jakarta-ee9+
      - konveyor.io/target=jakarta-ee
      - konveyor.io/target=eap8
      - konveyor.io/target=eap
      - konveyor.io/source
      incidents:
      - uri: file:///opt/input/source/target/classes/META-INF/persistence.xml
        message: Replace `http://xmlns.jcp.org/xml/ns/persistence` with `https://jakarta.ee/xml/ns/persistence` and change the schema version number
        codeSnip: |2
            1  <?xml version="1.0" encoding="UTF-8"?>
            2  <persistence version="2.1"
            3               xmlns="http://xmlns.jcp.org/xml/ns/persistence" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
            4               xsi:schemaLocation="
            5          http://xmlns.jcp.org/xml/ns/persistence
            6          http://xmlns.jcp.org/xml/ns/persistence/persistence_2_1.xsd">
            7      <persistence-unit name="primary">
            8          <jta-data-source>java:jboss/datasources/CoolstoreDS</jta-data-source>
            9          <properties>
           10              <property name="javax.persistence.schema-generation.database.action" value="none"/>
           11              <property name="hibernate.show_sql" value="false" />
           12          </properties>
           13      </persistence-unit>
           14  </persistence>
        lineNumber: 3
        variables:
          matchingText: http://xmlns.jcp.org/xml/ns/persistence

@jmle
Copy link
Contributor Author

jmle commented Nov 27, 2023

We should filter out results coming from target folders. I thought the folder was appearing due to our use of Maven for getting the dependency tree and downloading sources, but it looks like the language server does some sort of compilation and creates them.

aufi added a commit to aufi/analyzer-lsp that referenced this issue Dec 6, 2023
Updating filecontent condition to exclude files in "target"
subdirectory. This condition already used grep, so adding --exclude-dir
option there.

Fixes konveyor#358
aufi added a commit to aufi/analyzer-lsp that referenced this issue Dec 6, 2023
Updating filecontent condition to exclude files in "target"
subdirectory. This condition already used grep, so adding --exclude-dir
option there.

Fixes konveyor#358

Signed-off-by: Marek Aufart <[email protected]>
@rromannissen
Copy link

@jmle @shawn-hurley @pranavgaikwad I've been consistently reproducing this with multiple apps. An example of that can be seen with the properties file from the tackle-testapp application, the Hardcoded IP issue gets triggered twice, both in the application directory and under the target directory.

@dymurray dymurray added this to the v0.5.0 milestone Jun 26, 2024
@dymurray dymurray added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Jun 26, 2024
@konveyor-ci-bot konveyor-ci-bot bot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Jun 26, 2024
@konveyor-ci-bot
Copy link

This issue is currently awaiting triage.
If contributors determine this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.
The triage/accepted label can be added by org members.

@eemcmullan eemcmullan self-assigned this Jun 26, 2024
@shawn-hurley
Copy link
Contributor

I assume this is because of the builtin provider not excluding this directory, while the java LS and stuff is (AFAIK) now not searching there.

@eemcmullan if we can, we should wait for the PR that I have in flight because I am making some changes here.

@dymurray dymurray changed the title [BUG] Files in the target folder are being analyzed [BUG] Files in the target folder are being analyzed for builtin provider rules Jul 25, 2024
@dymurray dymurray modified the milestones: v0.5.0, v0.5.1, v0.6.0 Jul 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Projects
Status: 🏗 In progress
Development

Successfully merging a pull request may close this issue.

7 participants