Skip to content

Commit f9dd80b

Browse files
gabrielfeogabrielfeo
authored
Fix token reading in Linux (#56)
Closes #54 ### #54 Trying to use it in Linux with `GRADLE_ENTERPRISE_API_TOKEN` fails first due to having no `LOGNAME` variable: ``` null java.lang.NullPointerException at java.base/java.lang.ProcessBuilder.start(ProcessBuilder.java:1090) at java.base/java.lang.ProcessBuilder.start(ProcessBuilder.java:1071) at com.gabrielfeo.gradle.enterprise.api.internal.RealKeychain.get(Keychain.kt:18) at com.gabrielfeo.gradle.enterprise.api.Options$GradleEnterpriseInstanceOptions$token$1.invoke(Options.kt:56) at com.gabrielfeo.gradle.enterprise.api.Options$GradleEnterpriseInstanceOptions$token$1.invoke(Options.kt:55) at com.gabrielfeo.gradle.enterprise.api.internal.OkHttpClientKt.addNetworkInterceptors(OkHttpClient.kt:49) at com.gabrielfeo.gradle.enterprise.api.internal.OkHttpClientKt.buildOkHttpClient(OkHttpClient.kt:27) at com.gabrielfeo.gradle.enterprise.api.internal.OkHttpClientKt$okHttpClient$2.invoke(OkHttpClient.kt:16) at com.gabrielfeo.gradle.enterprise.api.internal.OkHttpClientKt$okHttpClient$2.invoke(OkHttpClient.kt:15) at kotlin.SynchronizedLazyImpl.getValue(LazyJVM.kt:74) at com.gabrielfeo.gradle.enterprise.api.internal.OkHttpClientKt.getOkHttpClient(OkHttpClient.kt:15) at com.gabrielfeo.gradle.enterprise.api.internal.RetrofitKt$retrofit$2.invoke(Retrofit.kt:15) at com.gabrielfeo.gradle.enterprise.api.internal.RetrofitKt$retrofit$2.invoke(Retrofit.kt:12) at kotlin.SynchronizedLazyImpl.getValue(LazyJVM.kt:74) at com.gabrielfeo.gradle.enterprise.api.internal.RetrofitKt.getRetrofit(Retrofit.kt:12) at com.gabrielfeo.gradle.enterprise.api.ApiKt$gradleEnterpriseApi$2.invoke(Api.kt:11) at com.gabrielfeo.gradle.enterprise.api.ApiKt$gradleEnterpriseApi$2.invoke(Api.kt:10) at kotlin.SynchronizedLazyImpl.getValue(LazyJVM.kt:74) at com.gabrielfeo.gradle.enterprise.api.ApiKt.getGradleEnterpriseApi(Api.kt:10) ``` After setting a dummy `LOGNAME`, it'll still fail because ProcessBuilder throws an `IOException` for command not found, instead of just setting status to 127: ``` Cannot run program "security": error=2, No such file or directory java.io.IOException: Cannot run program "security": error=2, No such file or directory at java.base/java.lang.ProcessBuilder.start(ProcessBuilder.java:1128) at java.base/java.lang.ProcessBuilder.start(ProcessBuilder.java:1071) at com.gabrielfeo.gradle.enterprise.api.internal.RealKeychain.get(Keychain.kt:18) at com.gabrielfeo.gradle.enterprise.api.Options$GradleEnterpriseInstanceOptions$token$1.invoke(Options.kt:56) at com.gabrielfeo.gradle.enterprise.api.Options$GradleEnterpriseInstanceOptions$token$1.invoke(Options.kt:55) at com.gabrielfeo.gradle.enterprise.api.internal.OkHttpClientKt.addNetworkInterceptors(OkHttpClient.kt:49) at com.gabrielfeo.gradle.enterprise.api.internal.OkHttpClientKt.buildOkHttpClient(OkHttpClient.kt:27) at com.gabrielfeo.gradle.enterprise.api.internal.OkHttpClientKt$okHttpClient$2.invoke(OkHttpClient.kt:16) at com.gabrielfeo.gradle.enterprise.api.internal.OkHttpClientKt$okHttpClient$2.invoke(OkHttpClient.kt:15) at kotlin.SynchronizedLazyImpl.getValue(LazyJVM.kt:74) at com.gabrielfeo.gradle.enterprise.api.internal.OkHttpClientKt.getOkHttpClient(OkHttpClient.kt:15) at com.gabrielfeo.gradle.enterprise.api.internal.RetrofitKt$retrofit$2.invoke(Retrofit.kt:15) at com.gabrielfeo.gradle.enterprise.api.internal.RetrofitKt$retrofit$2.invoke(Retrofit.kt:12) at kotlin.SynchronizedLazyImpl.getValue(LazyJVM.kt:74) at com.gabrielfeo.gradle.enterprise.api.internal.RetrofitKt.getRetrofit(Retrofit.kt:12) at com.gabrielfeo.gradle.enterprise.api.ApiKt$gradleEnterpriseApi$2.invoke(Api.kt:11) at com.gabrielfeo.gradle.enterprise.api.ApiKt$gradleEnterpriseApi$2.invoke(Api.kt:10) at kotlin.SynchronizedLazyImpl.getValue(LazyJVM.kt:74) at com.gabrielfeo.gradle.enterprise.api.ApiKt.getGradleEnterpriseApi(Api.kt:10) ```
1 parent 11f2722 commit f9dd80b

File tree

10 files changed

+145
-29
lines changed

10 files changed

+145
-29
lines changed

‎build.gradle.kts‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
@file:Suppress("UnstableApiUsage")
2+
13
import java.net.URL
24
import org.jetbrains.dokka.DokkaConfiguration.Visibility.PUBLIC
35
import org.jetbrains.dokka.gradle.DokkaTask
@@ -177,8 +179,23 @@ publishing {
177179
}
178180

179181
testing {
180-
suites.withType<JvmTestSuite> {
181-
useKotlinTest()
182+
suites {
183+
getByName<JvmTestSuite>("test") {
184+
dependencies {
185+
implementation("com.squareup.okhttp3:mockwebserver:4.11.0")
186+
implementation("com.squareup.okio:okio:3.3.0")
187+
implementation("org.jetbrains.kotlinx:kotlinx-coroutines-test:1.7.0")
188+
}
189+
}
190+
register<JvmTestSuite>("integrationTest") {
191+
dependencies {
192+
implementation(project())
193+
implementation("org.jetbrains.kotlinx:kotlinx-coroutines-test:1.7.0")
194+
}
195+
}
196+
withType<JvmTestSuite> {
197+
useKotlinTest()
198+
}
182199
}
183200
}
184201

@@ -200,7 +217,4 @@ dependencies {
200217
implementation("com.squareup.retrofit2:converter-moshi:2.9.0")
201218
implementation("com.squareup.retrofit2:converter-scalars:2.9.0")
202219
api("org.jetbrains.kotlinx:kotlinx-coroutines-core:1.7.0")
203-
testImplementation("com.squareup.okhttp3:mockwebserver:4.11.0")
204-
testImplementation("com.squareup.okio:okio:3.3.0")
205-
testImplementation("org.jetbrains.kotlinx:kotlinx-coroutines-test:1.7.0")
206220
}

‎src/integrationTest/kotlin/com/gabrielfeo/gradle/enterprise/api/internal/GradleEnterpriseIntegrationTest.kt‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
package com.gabrielfeo.gradle.enterprise.api.internal
2+
3+
import com.gabrielfeo.gradle.enterprise.api.gradleEnterpriseApi
4+
import com.gabrielfeo.gradle.enterprise.api.shutdown
5+
import kotlinx.coroutines.test.runTest
6+
import kotlin.test.Test
7+
import kotlin.test.assertEquals
8+
9+
class GradleEnterpriseIntegrationTest {
10+
11+
@Test
12+
fun canFetchBuilds() = runTest {
13+
val builds = gradleEnterpriseApi.getBuilds(since = 0, maxBuilds = 1)
14+
assertEquals(1, builds.size)
15+
shutdown()
16+
}
17+
}

‎src/integrationTest/kotlin/com/gabrielfeo/gradle/enterprise/api/internal/KeychainIntegrationTest.kt‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
package com.gabrielfeo.gradle.enterprise.api.internal
2+
3+
import kotlin.test.Test
4+
import kotlin.test.assertFalse
5+
6+
class KeychainIntegrationTest {
7+
8+
val keychain = RealKeychain(RealSystemProperties)
9+
10+
@Test
11+
fun getApiToken() {
12+
assertFalse(
13+
keychain["gradle-enterprise-api-token"].isNullOrEmpty(),
14+
"Keychain returned null or empty",
15+
)
16+
}
17+
}

‎src/main/kotlin/com/gabrielfeo/gradle/enterprise/api/Options.kt‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
package com.gabrielfeo.gradle.enterprise.api
44

55
import com.gabrielfeo.gradle.enterprise.api.internal.*
6+
import com.gabrielfeo.gradle.enterprise.api.internal.SystemProperties
67
import okhttp3.Dispatcher
78
import okhttp3.OkHttpClient
89
import java.io.File
@@ -11,7 +12,11 @@ import kotlin.time.Duration.Companion.days
1112
/**
1213
* The global [Options] instance.
1314
*/
14-
val options = Options(env = RealEnv, keychain = RealKeychain(RealEnv))
15+
val options = Options(
16+
env = RealEnv,
17+
keychain = RealKeychain(RealSystemProperties),
18+
systemProperties = RealSystemProperties,
19+
)
1520

1621
/**
1722
* Library configuration options. Should not be changed after accessing the [gradleEnterpriseApi]
@@ -21,10 +26,11 @@ val options = Options(env = RealEnv, keychain = RealKeychain(RealEnv))
2126
*/
2227
class Options internal constructor(
2328
env: Env,
29+
systemProperties: SystemProperties,
2430
keychain: Keychain,
2531
) {
2632

27-
val gradleEnterpriseInstance = GradleEnterpriseInstanceOptions(env, keychain)
33+
val gradleEnterpriseInstance = GradleEnterpriseInstanceOptions(env, keychain, systemProperties)
2834
val httpClient = HttpClientOptions(env)
2935
val cache = CacheOptions(env)
3036
val debugging = DebuggingOptions(env)
@@ -37,6 +43,7 @@ class Options internal constructor(
3743
class GradleEnterpriseInstanceOptions internal constructor(
3844
private val env: Env,
3945
private val keychain: Keychain,
46+
private val systemProperties: SystemProperties,
4047
) {
4148

4249
/**
@@ -52,10 +59,13 @@ class Options internal constructor(
5259
* Provides the access token for a Gradle Enterprise API instance. By default, uses keychain entry
5360
* `gradle-enterprise-api-token` or environment variable `GRADLE_ENTERPRISE_API_TOKEN`.
5461
*/
55-
var token: () -> String = {
56-
keychain["gradle-enterprise-api-token"]
57-
?: env["GRADLE_ENTERPRISE_API_TOKEN"]
58-
?: error("GRADLE_ENTERPRISE_API_TOKEN is required")
62+
var token: () -> String = token@{
63+
if (systemProperties["os.name"] == "Mac OS X") {
64+
keychain["gradle-enterprise-api-token"]?.takeIf { it.isNotBlank() }?.let {
65+
return@token it
66+
}
67+
}
68+
env["GRADLE_ENTERPRISE_API_TOKEN"] ?: error("GRADLE_ENTERPRISE_API_TOKEN is required")
5969
}
6070
}
6171

‎src/main/kotlin/com/gabrielfeo/gradle/enterprise/api/internal/Keychain.kt‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,13 @@ interface Keychain {
99
}
1010

1111
class RealKeychain(
12-
private val env: Env,
12+
private val systemProperties: SystemProperties,
1313
) : Keychain {
1414
override fun get(entry: String): String? {
15-
val login = env["LOGNAME"]
15+
val login = systemProperties["user.name"] ?: let {
16+
Logger.getGlobal().log(Level.INFO, "Failed to get key from keychain (null user.name)")
17+
return null
18+
}
1619
val process = ProcessBuilder(
1720
"security", "find-generic-password", "-w", "-a", login, "-s", entry
1821
).start()

‎src/main/kotlin/com/gabrielfeo/gradle/enterprise/api/internal/SystemProperties.kt‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
package com.gabrielfeo.gradle.enterprise.api.internal
2+
3+
interface SystemProperties {
4+
operator fun get(name: String): String?
5+
}
6+
7+
object RealSystemProperties : SystemProperties {
8+
override fun get(name: String): String? = System.getProperty(name)
9+
}

‎src/test/kotlin/com/gabrielfeo/gradle/enterprise/api/OkHttpClientTest.kt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package com.gabrielfeo.gradle.enterprise.api
22

33
import com.gabrielfeo.gradle.enterprise.api.internal.FakeEnv
44
import com.gabrielfeo.gradle.enterprise.api.internal.FakeKeychain
5+
import com.gabrielfeo.gradle.enterprise.api.internal.FakeSystemProperties
56
import com.gabrielfeo.gradle.enterprise.api.internal.auth.HttpBearerAuth
67
import com.gabrielfeo.gradle.enterprise.api.internal.buildOkHttpClient
78
import com.gabrielfeo.gradle.enterprise.api.internal.caching.CacheEnforcingInterceptor
@@ -90,7 +91,7 @@ class OkHttpClientTest {
9091
env["GRADLE_ENTERPRISE_API_TOKEN"] = "example-token"
9192
if ("GRADLE_ENTERPRISE_API_URL" !in env)
9293
env["GRADLE_ENTERPRISE_API_URL"] = "example-url"
93-
val options = Options(env, FakeKeychain()).apply {
94+
val options = Options(env, FakeSystemProperties.macOs, FakeKeychain()).apply {
9495
clientBuilder?.let {
9596
httpClient.clientBuilder = { it }
9697
}

‎src/test/kotlin/com/gabrielfeo/gradle/enterprise/api/OptionsTest.kt‎

Lines changed: 40 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
package com.gabrielfeo.gradle.enterprise.api
22

3-
import com.gabrielfeo.gradle.enterprise.api.internal.Env
4-
import com.gabrielfeo.gradle.enterprise.api.internal.FakeEnv
5-
import com.gabrielfeo.gradle.enterprise.api.internal.FakeKeychain
3+
import com.gabrielfeo.gradle.enterprise.api.internal.*
64
import org.junit.jupiter.api.assertDoesNotThrow
75
import kotlin.test.Test
86
import kotlin.test.assertEquals
@@ -13,7 +11,7 @@ class OptionsTest {
1311

1412
@Test
1513
fun `Given no URL set in env, url() fails`() {
16-
val options = Options(FakeEnv(), FakeKeychain())
14+
val options = Options(FakeEnv(), FakeSystemProperties.macOs, FakeKeychain())
1715
assertFails {
1816
options.gradleEnterpriseInstance.url()
1917
}
@@ -23,34 +21,64 @@ class OptionsTest {
2321
fun `Given URL set in env, url() returns env URL`() {
2422
val options = Options(
2523
FakeEnv("GRADLE_ENTERPRISE_API_URL" to "https://example.com/api/"),
24+
FakeSystemProperties.macOs,
2625
FakeKeychain(),
2726
)
2827
assertEquals("https://example.com/api/", options.gradleEnterpriseInstance.url())
2928
}
3029

3130
@Test
32-
fun `Token from keychain is preferred`() {
31+
fun `Given macOS and keychain token, keychain token used`() {
3332
val options = Options(
3433
keychain = FakeKeychain("gradle-enterprise-api-token" to "foo"),
3534
env = FakeEnv("GRADLE_ENTERPRISE_API_TOKEN" to "bar"),
35+
systemProperties = FakeSystemProperties.macOs,
3636
)
3737
assertEquals("foo", options.gradleEnterpriseInstance.token())
3838
}
3939

4040
@Test
41-
fun `Token from env is fallback`() {
41+
fun `Given macOS but no keychain token, env token used`() {
4242
val options = Options(
4343
keychain = FakeKeychain(),
4444
env = FakeEnv("GRADLE_ENTERPRISE_API_TOKEN" to "bar"),
45+
systemProperties = FakeSystemProperties.macOs,
4546
)
4647
assertEquals("bar", options.gradleEnterpriseInstance.token())
4748
}
4849

4950
@Test
50-
fun `Token from keychain or env is required`() {
51+
fun `Given Linux, keychain never tried and env token used`() {
52+
val options = Options(
53+
env = FakeEnv("GRADLE_ENTERPRISE_API_TOKEN" to "bar"),
54+
systemProperties = FakeSystemProperties.linux,
55+
keychain = object : Keychain {
56+
override fun get(entry: String): String? {
57+
error("Error: Tried to access macOS keychain in Linux")
58+
}
59+
},
60+
)
61+
assertEquals("bar", options.gradleEnterpriseInstance.token())
62+
}
63+
64+
@Test
65+
fun `Given macOS and no token anywhere, fails`() {
66+
val options = Options(
67+
keychain = FakeKeychain(),
68+
env = FakeEnv(),
69+
systemProperties = FakeSystemProperties.macOs,
70+
)
71+
assertFails {
72+
options.gradleEnterpriseInstance.token()
73+
}
74+
}
75+
76+
@Test
77+
fun `Given Linux and no env token, fails`() {
5178
val options = Options(
5279
keychain = FakeKeychain(),
5380
env = FakeEnv(),
81+
systemProperties = FakeSystemProperties.linux,
5482
)
5583
assertFails {
5684
options.gradleEnterpriseInstance.token()
@@ -62,6 +90,7 @@ class OptionsTest {
6290
val options = Options(
6391
keychain = FakeKeychain(),
6492
env = FakeEnv("GRADLE_ENTERPRISE_API_MAX_CONCURRENT_REQUESTS" to "1"),
93+
systemProperties = FakeSystemProperties.macOs,
6594
)
6695
assertDoesNotThrow {
6796
options.httpClient.maxConcurrentRequests
@@ -70,7 +99,7 @@ class OptionsTest {
7099

71100
@Test
72101
fun `default longTermCacheUrlPattern matches attributes URLs`() {
73-
val options = Options(FakeEnv(), FakeKeychain())
102+
val options = Options(FakeEnv(), FakeSystemProperties.macOs, FakeKeychain())
74103
options.cache.longTermCacheUrlPattern.assertMatches(
75104
"https://ge.gradle.org/api/builds/tgnsqkb2rhlni/gradle-attributes",
76105
"https://ge.gradle.org/api/builds/tgnsqkb2rhlni/maven-attributes",
@@ -79,7 +108,7 @@ class OptionsTest {
79108

80109
@Test
81110
fun `default longTermCacheUrlPattern matches build cache performance URLs`() {
82-
val options = Options(FakeEnv(), FakeKeychain())
111+
val options = Options(FakeEnv(), FakeSystemProperties.macOs, FakeKeychain())
83112
options.cache.longTermCacheUrlPattern.assertMatches(
84113
"https://ge.gradle.org/api/builds/tgnsqkb2rhlni/gradle-build-cache-performance",
85114
"https://ge.gradle.org/api/builds/tgnsqkb2rhlni/maven-build-cache-performance",
@@ -88,7 +117,7 @@ class OptionsTest {
88117

89118
@Test
90119
fun `default shortTermCacheUrlPattern matches builds URLs`() {
91-
val options = Options(FakeEnv(), FakeKeychain())
120+
val options = Options(FakeEnv(), FakeSystemProperties.macOs, FakeKeychain())
92121
options.cache.shortTermCacheUrlPattern.assertMatches(
93122
"https://ge.gradle.org/api/builds?since=0",
94123
"https://ge.gradle.org/api/builds?since=0&maxBuilds=2",
@@ -99,6 +128,7 @@ class OptionsTest {
99128
fun `Given timeout set in env, readTimeoutMillis returns env value`() {
100129
val options = Options(
101130
FakeEnv("GRADLE_ENTERPRISE_API_READ_TIMEOUT_MILLIS" to "100000"),
131+
FakeSystemProperties.macOs,
102132
FakeKeychain(),
103133
)
104134
assertEquals(100_000L, options.httpClient.readTimeoutMillis)

‎src/test/kotlin/com/gabrielfeo/gradle/enterprise/api/RetrofitTest.kt‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
11
package com.gabrielfeo.gradle.enterprise.api
22

3-
import com.gabrielfeo.gradle.enterprise.api.internal.FakeEnv
4-
import com.gabrielfeo.gradle.enterprise.api.internal.FakeKeychain
3+
import com.gabrielfeo.gradle.enterprise.api.internal.*
54
import com.gabrielfeo.gradle.enterprise.api.internal.auth.HttpBearerAuth
6-
import com.gabrielfeo.gradle.enterprise.api.internal.buildOkHttpClient
7-
import com.gabrielfeo.gradle.enterprise.api.internal.buildRetrofit
85
import com.gabrielfeo.gradle.enterprise.api.internal.caching.CacheEnforcingInterceptor
96
import com.gabrielfeo.gradle.enterprise.api.internal.caching.CacheHitLoggingInterceptor
107
import com.squareup.moshi.Moshi
@@ -38,7 +35,7 @@ class RetrofitTest {
3835
val env = FakeEnv(*envVars)
3936
if ("GRADLE_ENTERPRISE_API_TOKEN" !in env)
4037
env["GRADLE_ENTERPRISE_API_TOKEN"] = "example-token"
41-
val options = Options(env, FakeKeychain())
38+
val options = Options(env, FakeSystemProperties.macOs, FakeKeychain())
4239
return buildRetrofit(
4340
options = options,
4441
client = buildOkHttpClient(options),

‎src/test/kotlin/com/gabrielfeo/gradle/enterprise/api/internal/FakeSystemProperties.kt‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
package com.gabrielfeo.gradle.enterprise.api.internal
2+
3+
class FakeSystemProperties(
4+
vararg vars: Pair<String, String?>,
5+
) : SystemProperties {
6+
7+
companion object {
8+
val macOs = FakeSystemProperties("os.name" to "Mac OS X")
9+
val linux = FakeSystemProperties("os.name" to "Linux")
10+
}
11+
12+
private val vars = vars.toMap(HashMap())
13+
14+
override fun get(name: String) = vars[name]
15+
16+
operator fun set(name: String, value: String?) = vars.put(name, value)
17+
operator fun contains(name: String) = name in vars
18+
}

0 commit comments

Comments
 (0)