-
Notifications
You must be signed in to change notification settings - Fork 92
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
Support development dependencies for the Gradle detector #878
Conversation
Lack of development dependency detection for Gradle is a problem for Android teams, especially in the context of Component Governance alerts. Unfortunately Gradle doesn't provide enough information to definitively identify dev dependencies in all cases, so manual configuration is required. This change adds dev dependency classification through two mechanisms 1. `buildscript-gradle.lockfile` and `settings-gradle.lockfile` contain only build-system dependencies, so always classify these as development dependencies. 2. Processing based on two new environment variables: `GRADLE_PROD_CONFIGURATIONS_REGEX` and `GRADLE_DEV_CONFIGURATIONS_REGEX`. Gradle lockfiles indicate which Gradle configuration(s) each dependency is required by. `GRADLE_PROD_CONFIGURATIONS_REGEX` allows specifying production configurations explicitly. All other configurations are considered development. Alternately, dev configurations may be specified in `GRADLE_DEV_CONFIGURATIONS_REGEX` and all others are considered production.
CG is auto-injected into all production pipelines, so I fear that maintaining an accurate configuration list in an environment variable will be near-impossible. CG operates on lockfiles as checked-in files, so I think ultimately the config needs to be a checked-in file as well. |
src/Microsoft.ComponentDetection.Detectors/gradle/GradleComponentDetector.cs
Outdated
Show resolved
Hide resolved
I'm aware, but not sure why we couldn't set the variable in all production pipelines too?
I'd like input from the CG/component-detection owners before proceeding further here. There are existing configurations for specific detectors with environment variables, and I don't see any precedent for file-based configuration in this tool. I'm happy to go either direction based on guidance/preference of the owners here. |
src/Microsoft.ComponentDetection.Detectors/gradle/GradleComponentDetector.cs
Show resolved
Hide resolved
src/Microsoft.ComponentDetection.Detectors/gradle/GradleComponentDetector.cs
Outdated
Show resolved
Hide resolved
Thanks for the review @melotic. Before I make changes to address your comments on the environment variable access and naming, could you or @JamieMagee weigh in on Ryan's suggestion to use a configuration file instead of environment variables? I'm happy to go with either approach. I agree with @rygo-msft 's point on the benefit of a configuration file, but am not sure if it would be too inconsistent with the rest of the component-detection tool. |
Hi @melotic, @JamieMagee -- any feedback on the configuration approach discussion (environment vs file)? |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #878 +/- ##
=====================================
Coverage 75.3% 75.4%
=====================================
Files 236 236
Lines 10353 10386 +33
Branches 1025 1032 +7
=====================================
+ Hits 7800 7833 +33
Misses 2269 2269
Partials 284 284 ☔ View full report in Codecov by Sentry. |
I'm still unclear how this list of developer configurations will be built and maintained in practice. A typical android project will have 20+ different configurations and that number grows even large when build flavors are used. Because lockfiles are static files, they will be scanned even for pipelines unrelated to the gradle build. These config values will need to be copied and initialized in every pipeline somehow. This is particularly difficult to achieve as static analysis jobs are typically injected without pipeline author input. There are cases where pipeline authors cannot provide configuration these injected jobs. |
I would still like to hear from the repo maintainers if they would accept a change that read from a configuration file instead of environment variables. I agree that would be somewhat preferable in this case. Ryan, can you give an example of a case where the pipeline authors cannot provide configuration via environment variables though? I was assuming we would just run the task setting up the environment variables before the component detection task. |
Governed Templates inject static analysis jobs that run standardized task sets and do not take user provided tasks. |
We don't need to run a gradle build, just to be able to set up environment variables. But I wasn't familiar with the auto-injected static analysis jobs, I agree that's a concern and a good motivation for allowing configuration by file instead of environment variable. I'll pause here to see if Anna, Jamie or others can weigh in. |
I would prefer a configuration file based approach in the long term, but I think it's acceptable while we iterate on the idea of how this kind of configuration should be maintained. In the future I'd like to see a In the meantime, let me go ahead and review the rest. |
src/Microsoft.ComponentDetection.Detectors/gradle/GradleComponentDetector.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.ComponentDetection.Detectors/gradle/GradleComponentDetector.cs
Show resolved
Hide resolved
src/Microsoft.ComponentDetection.Detectors/gradle/GradleComponentDetector.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.ComponentDetection.Detectors/gradle/GradleComponentDetector.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.ComponentDetection.Detectors/gradle/GradleComponentDetector.cs
Outdated
Show resolved
Hide resolved
👋 Hi! It looks like you modified some files in the
If none of the above scenarios apply, feel free to ignore this comment 🙂 |
Lack of development dependency detection for Gradle is a problem for Android teams, especially in the context of Component Governance alerts. Unfortunately Gradle doesn't provide enough information to definitively identify dev dependencies in all cases, so manual configuration is required. This change adds dev dependency classification through two mechanisms
buildscript-gradle.lockfile
andsettings-gradle.lockfile
contain only build-system dependencies, so always classify these as development dependencies.GRADLE_PROD_CONFIGURATIONS_REGEX
andGRADLE_DEV_CONFIGURATIONS_REGEX
. Gradle lockfiles indicate which Gradle configuration(s) each dependency is required by.GRADLE_PROD_CONFIGURATIONS_REGEX
allows specifying production configurations explicitly. All other configurations are considered development. Alternately, dev configurations may be specified inGRADLE_DEV_CONFIGURATIONS_REGEX
and all others are considered production.