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

Refactor: Merge TokenFile auth with refresh auth #3817

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.time.Instant;
import java.util.function.Supplier;

import io.kubernetes.client.util.exception.TokenAquisitionException;
import okhttp3.Interceptor;
import okhttp3.OkHttpClient;
import okhttp3.Request;
Expand All @@ -28,7 +29,6 @@
// TODO: prefer OpenAPI backed Auentication once it is available. see details in
// https://github.com/OpenAPITools/openapi-generator/pull/6036. currently, the
// workaround is to hijack the http request.
// TODO: Merge this with TokenFileAuthentication.
public class RefreshAuthentication implements Authentication, Interceptor {
private Instant expiry;
private Duration refreshPeriod;
Expand All @@ -43,14 +43,22 @@ public RefreshAuthentication(Supplier<String> tokenSupplier, Duration refreshPer
public RefreshAuthentication(Supplier<String> tokenSupplier, Duration refreshPeriod, Clock clock) {
this.expiry = Instant.MIN;
this.refreshPeriod = refreshPeriod;
this.token = tokenSupplier.get();
try {
this.token = tokenSupplier.get();
} catch (RuntimeException e) {
throw new TokenAquisitionException(e);
}
this.tokenSupplier = tokenSupplier;
this.clock = clock;
}

private String getToken() {
if (Instant.now(this.clock).isAfter(this.expiry)) {
this.token = tokenSupplier.get();
try {
this.token = tokenSupplier.get();
} catch (RuntimeException e) {
throw new TokenAquisitionException(e);
}
expiry = Instant.now(this.clock).plusSeconds(refreshPeriod.toSeconds());
}
return this.token;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,63 +12,32 @@
*/
package io.kubernetes.client.util.credentials;

import io.kubernetes.client.openapi.ApiClient;
import java.io.IOException;
import java.nio.charset.Charset;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.time.Instant;
import okhttp3.Interceptor;
import okhttp3.OkHttpClient;
import okhttp3.Request;
import okhttp3.Response;
import java.time.Duration;

// TODO: prefer OpenAPI backed Auentication once it is available. see details in
// https://github.com/OpenAPITools/openapi-generator/pull/6036. currently, the
// workaround is to hijack the http request.
public class TokenFileAuthentication implements Authentication, Interceptor {
private String file;
private String token;
private Instant expiry;

public class TokenFileAuthentication extends RefreshAuthentication {
public TokenFileAuthentication(String file) {
this.expiry = Instant.MIN;
this.file = file;
}

private String getToken() {
if (Instant.now().isAfter(this.expiry)) {
try {
this.token =
new String(Files.readAllBytes(Paths.get(this.file)), Charset.defaultCharset()).trim();
expiry = Instant.now().plusSeconds(60);
} catch (IOException ie) {
throw new RuntimeException("Cannot read file: " + this.file);
}
}

return this.token;
this(file, Duration.ofMinutes(1));
}

public void setExpiry(Instant expiry) {
this.expiry = expiry;
public TokenFileAuthentication(String file, Duration refreshPeriod) {
super(() -> {
return getToken(file);
}, refreshPeriod);
}

public void setFile(String file) {
this.file = file;
}

@Override
public void provide(ApiClient client) {
OkHttpClient withInterceptor = client.getHttpClient().newBuilder().addInterceptor(this).build();
client.setHttpClient(withInterceptor);
private static String getToken(String file) {
try {
return new String(Files.readAllBytes(Paths.get(file)), Charset.defaultCharset()).trim();
} catch (IOException e) {
throw new RuntimeException("Cannot read file: " + file);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably convert this to some sort of non-RuntimeException like "FailedTokenAquisitionException" and handle that inside the RefreshAuthentication.java

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds like a good idea but I ended up adding the exception conversion as a runtime/unchecked exception in the parent class RefreshAuthentication. I also tried making it a checked exception in the token supplier function however it will break backward compatibility so I implemented the unchecked exception in the new commit. one example that compatibility would be broken is here where we're loading the token eagerly in RefreshAuthentication :

https://github.com/kubernetes-client/java/blob/master/util/src/main/java/io/kubernetes/client/util/credentials/RefreshAuthentication.java#L43

so the new checked exception would have to be added to the constructor which will require all places referencing it adding explicit handling the checked exception. does that make sense?

}
}

@Override
public Response intercept(Interceptor.Chain chain) throws IOException {
Request request = chain.request();
Request newRequest;
newRequest = request.newBuilder().header("Authorization", "Bearer " + getToken()).build();
return chain.proceed(newRequest);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/*
Copyright 2024 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package io.kubernetes.client.util.exception;

public class TokenAquisitionException extends RuntimeException {
public TokenAquisitionException(Exception exception) {
super(exception);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,17 +42,19 @@ class TokenFileAuthenticationTest {
static WireMockExtension apiServer =
WireMockExtension.newInstance().options(options().dynamicPort()).build();

private ApiClient apiClient;

@BeforeEach
void setup() {
final ApiClient client = new ApiClient();
client.setBasePath("http://localhost:" + apiServer.getPort());
this.auth = new TokenFileAuthentication(SERVICEACCOUNT_TOKEN1_PATH);
this.auth.provide(client);
Configuration.setDefaultApiClient(client);
this.apiClient = new ApiClient();
this.apiClient.setBasePath("http://localhost:" + apiServer.getPort());
Configuration.setDefaultApiClient(this.apiClient);
}

@Test
void tokenProvided() throws ApiException {
void tokenProvidedTokenNormalFileReadShouldWork() throws ApiException {
Authentication authn = new TokenFileAuthentication(SERVICEACCOUNT_TOKEN1_PATH);
authn.provide(this.apiClient);
apiServer.stubFor(
get(urlPathEqualTo("/api/v1/pods")).willReturn(okForContentType("application/json",
"{\"items\":[]}")));
Expand All @@ -63,19 +65,6 @@ void tokenProvided() throws ApiException {
1,
getRequestedFor(urlPathEqualTo("/api/v1/pods"))
.withHeader("Authorization", equalTo("Bearer token1")));

this.auth.setFile(SERVICEACCOUNT_TOKEN2_PATH);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these unit tests are no longer needed as it was originally for testing token expiry/rotation behavior. now it's decoupled/delegated to the RefreshAuthentication class.

api.listPodForAllNamespaces().execute();
apiServer.verify(
2,
getRequestedFor(urlPathEqualTo("/api/v1/pods"))
.withHeader("Authorization", equalTo("Bearer token1")));

this.auth.setExpiry(Instant.now().minusSeconds(1));
api.listPodForAllNamespaces().execute();
apiServer.verify(
1,
getRequestedFor(urlPathEqualTo("/api/v1/pods"))
.withHeader("Authorization", equalTo("Bearer token2")));
}

}
Loading