Skip to content

Commit

Permalink
refactor (jkube-kit/config/resource) : Deprecate jkube.io annotatio…
Browse files Browse the repository at this point in the history
…n prefix for JKubeAnnotations (eclipse-jkube#1273)

+ Deprecate `jkube.io` annotation prefix in favor of `jkube.eclipse.org`
+ Add `jkube.useOldJKubePrefix` flag to switch to `jkube.io`
  annotation prefix in case any user is interested in this.

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
  • Loading branch information
rohanKanojia committed Jan 10, 2023
1 parent 3204f60 commit 3be8092
Show file tree
Hide file tree
Showing 12 changed files with 138 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -36,21 +36,36 @@ public enum JKubeAnnotations {
SCM_TAG("scm-tag"),
SCM_URL("scm-url"),

TARGET_PLATFORM("target-platform");
TARGET_PLATFORM("target-platform"),
ICON_URL("iconUrl");

private static final String JKUBE_ANNOTATION_PREFIX = "jkube.eclipse.org";
/**
* @deprecated in favor of <code>jkube.eclipse.org</code>
*/
@Deprecated
private static final String DEPRECATED_JKUBE_ANNOTATION_PREFIX = "jkube.io";

private final String annotation;

JKubeAnnotations(String anno) {
this.annotation = "jkube.io/" + anno;
this.annotation = "/" + anno;
}

public String value() {
return annotation;
public String value(boolean useDeprecatedPrefix) {
return getAnnotationPrefix(useDeprecatedPrefix) + annotation;
}

@Override
public String toString() {
return value();
return value(false);
}

public static String getAnnotationPrefix(boolean useDeprecatedPrefix) {
if (useDeprecatedPrefix) {
return DEPRECATED_JKUBE_ANNOTATION_PREFIX;
}
return JKUBE_ANNOTATION_PREFIX;
}
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/**
* Copyright (c) 2019 Red Hat, Inc.
* This program and the accompanying materials are made
* available under the terms of the Eclipse Public License 2.0
* which is available at:
*
* https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* Red Hat, Inc. - initial API and implementation
*/
package org.eclipse.jkube.kit.config.resource;

import org.junit.jupiter.api.Test;

import static org.assertj.core.api.Assertions.assertThat;

class JKubeAnnotationsTest {
@Test
void value_withUseDeprecatedPrefixFalse_shouldReturnJKubeEclipseOrgPrefix() {
// Given + When
String result = JKubeAnnotations.GIT_COMMIT.value(false);
// Then
assertThat(result).isEqualTo("jkube.eclipse.org/git-commit");
}

@Test
void value_withUseDeprecatedPrefixTrue_shouldReturnJKubeIoPrefix() {
// Given + When
String result = JKubeAnnotations.GIT_COMMIT.value(true);
// Then
assertThat(result).isEqualTo("jkube.io/git-commit");
}

@Test
void toString_whenInvoked_shouldUseJkubeEclipseOrgPrefix() {
// Given + When
String result = JKubeAnnotations.GIT_BRANCH.toString();
// Then
assertThat(result).isEqualTo("jkube.eclipse.org/git-branch");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ public class BaseEnricher implements Enricher {
public static final String JKUBE_ENFORCED_REPLICAS = "jkube.replicas";
public static final String JKUBE_DEFAULT_IMAGE_PULL_POLICY = "IfNotPresent";
public static final String JKUBE_ENFORCED_IMAGE_PULL_POLICY = "jkube.imagePullPolicy";
private static final String JKUBE_USE_OLD_PREFIX = "jkube.useOldJKubePrefix";

private final EnricherConfig config;
private final String name;
Expand Down Expand Up @@ -230,6 +231,10 @@ protected void setProcessingInstruction(String key, List<String> containerNames)
enricherContext.setProcessingInstructions(processingInstructionsMap);
}

protected boolean shouldUseOldJkubePrefix() {
return getValueFromConfig(JKUBE_USE_OLD_PREFIX, false);
}

/**
* Getting a property value from configuration
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,22 @@ void getImagePullPolicy_whenPullPolicySpecifiedViaProperty_shouldReturnPullPolic
// Then
assertThat(value).isEqualTo("Always");
}

@Test
void shouldUseDeprecatedAnnotations_whenPropertyProvided_thenReturnTrue() {
// Given
when(context.getProperty("jkube.useOldJKubePrefix")).thenReturn("true");
// When
boolean value = baseEnricher.shouldUseOldJkubePrefix();
// Then
assertThat(value).isTrue();
}

@Test
void shouldUseDeprecatedAnnotations_whenNothingProvided_thenReturnFalse() {
// Given + When
boolean value = baseEnricher.shouldUseOldJkubePrefix();
// Then
assertThat(value).isFalse();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ public GitEnricher(JKubeEnricherContext buildContext) {

private Map<String, String> getAnnotations(PlatformMode platformMode) {
final Map<String, String> annotations = new HashMap<>();
boolean useDeprecatedAnnotations = shouldUseOldJkubePrefix();
if (GitUtil.findGitFolder(getContext().getProjectDirectory()) != null) {
try (Repository repository = GitUtil.getGitRepository(getContext().getProjectDirectory())) {
// Git annotations (if git is used as SCM)
Expand All @@ -64,7 +65,7 @@ private Map<String, String> getAnnotations(PlatformMode platformMode) {
log.warn("Could not detect any git remote");
}

annotations.putAll(getAnnotations(platformMode, gitRemoteUrl, repository.getBranch(), GitUtil.getGitCommitId(repository)));
annotations.putAll(getAnnotations(platformMode, gitRemoteUrl, repository.getBranch(), GitUtil.getGitCommitId(repository), useDeprecatedAnnotations));
}
return annotations;
} catch (IOException | GitAPIException e) {
Expand Down Expand Up @@ -135,11 +136,11 @@ public void visit(JobBuilder builder) {
});
}

protected static Map<String, String> getAnnotations(PlatformMode platformMode, String gitRemoteUrl, String branch, String commitId) {
protected static Map<String, String> getAnnotations(PlatformMode platformMode, String gitRemoteUrl, String branch, String commitId, boolean useDeprecatedAnnotations) {
Map<String, String> annotationsToBeAdded = new HashMap<>();
annotationsToBeAdded.putAll(addAnnotation(JKubeAnnotations.GIT_BRANCH.value(), branch));
annotationsToBeAdded.putAll(addAnnotation(JKubeAnnotations.GIT_COMMIT.value(), commitId));
annotationsToBeAdded.putAll(addAnnotation(JKubeAnnotations.GIT_URL.value(), gitRemoteUrl));
annotationsToBeAdded.putAll(addAnnotation(JKubeAnnotations.GIT_BRANCH.value(useDeprecatedAnnotations), branch));
annotationsToBeAdded.putAll(addAnnotation(JKubeAnnotations.GIT_COMMIT.value(useDeprecatedAnnotations), commitId));
annotationsToBeAdded.putAll(addAnnotation(JKubeAnnotations.GIT_URL.value(useDeprecatedAnnotations), gitRemoteUrl));
if (platformMode.equals(PlatformMode.openshift)) {
annotationsToBeAdded.putAll(addAnnotation(OpenShiftAnnotations.VCS_URI.value(), gitRemoteUrl));
annotationsToBeAdded.putAll(addAnnotation(OpenShiftAnnotations.VCS_REF.value(), branch));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ public void visit(JobBuilder builder) {

private Map<String, String> getAnnotations() {
Map<String, String> annotations = new HashMap<>();
boolean useDeprecatedAnnotations = shouldUseOldJkubePrefix();

if (getContext() instanceof JKubeEnricherContext) {
JKubeEnricherContext jkubeEnricherContext = (JKubeEnricherContext) getContext();
Expand All @@ -119,8 +120,8 @@ private Map<String, String> getAnnotations() {
String system = rootProject.getIssueManagementSystem();
String url = rootProject.getIssueManagementUrl();
if (StringUtils.isNotEmpty(system) && StringUtils.isNotEmpty(url)) {
annotations.put(JKubeAnnotations.ISSUE_SYSTEM.value(), system);
annotations.put(JKubeAnnotations.ISSUE_TRACKER_URL.value(), url);
annotations.put(JKubeAnnotations.ISSUE_SYSTEM.value(useDeprecatedAnnotations), system);
annotations.put(JKubeAnnotations.ISSUE_TRACKER_URL.value(useDeprecatedAnnotations), url);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ public MavenScmEnricher(JKubeEnricherContext buildContext) {

private Map<String, String> getAnnotations() {
Map<String, String> annotations = new HashMap<>();
boolean useDeprecatedAnnotations = shouldUseOldJkubePrefix();

if (getContext() instanceof JKubeEnricherContext) {
JKubeEnricherContext jkubeEnricherContext = (JKubeEnricherContext) getContext();
Expand All @@ -63,10 +64,10 @@ private Map<String, String> getAnnotations() {
String tag = rootProject.getScmTag();

if (StringUtils.isNotEmpty(tag)) {
annotations.put(JKubeAnnotations.SCM_TAG.value(), tag);
annotations.put(JKubeAnnotations.SCM_TAG.value(useDeprecatedAnnotations), tag);
}
if (StringUtils.isNotEmpty(url)) {
annotations.put(JKubeAnnotations.SCM_URL.value(), url);
annotations.put(JKubeAnnotations.SCM_URL.value(useDeprecatedAnnotations), url);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import lombok.AllArgsConstructor;
import lombok.Getter;
import org.eclipse.jkube.kit.common.Configs;
import org.eclipse.jkube.kit.config.resource.JKubeAnnotations;
import org.eclipse.jkube.kit.config.resource.PlatformMode;
import org.eclipse.jkube.kit.enricher.api.BaseEnricher;
import org.eclipse.jkube.kit.enricher.api.JKubeEnricherContext;
Expand Down Expand Up @@ -54,9 +55,9 @@ public class AutoTLSEnricher extends BaseEnricher {
private enum Config implements Configs.Config {

TLS_SECRET_NAME("tlsSecretName", null),
TLS_SECRET_VOLUME_MOUNT_POINT("tlsSecretVolumeMountPoint", "/var/run/secrets/jkube.io/tls-pem"),
TLS_SECRET_VOLUME_MOUNT_POINT("tlsSecretVolumeMountPoint", "/var/run/secrets/%s/tls-pem"),
TLS_SECRET_VOLUME_NAME("tlsSecretVolumeName", "tls-pem"),
JKS_VOLUME_MOUNT_POINT("jksVolumeMountPoint", "/var/run/secrets/jkube.io/tls-jks"),
JKS_VOLUME_MOUNT_POINT("jksVolumeMountPoint", "/var/run/secrets/%s/tls-jks"),
JKS_VOLUME_NAME("jksVolumeName", "tls-jks"),
PEM_TO_JKS_INIT_CONTAINER_IMAGE("pemToJKSInitContainerImage", "jimmidyson/pemtokeystore:v0.1.0"),
PEM_TO_JKS_INIT_CONTAINER_NAME("pemToJKSInitContainerName", "tls-jks-converter"),
Expand Down Expand Up @@ -156,15 +157,17 @@ private boolean isVolumeAlreadyExists(List<Volume> volumes, String volumeName) {
public void visit(ContainerBuilder builder) {
String tlsSecretVolumeName = getConfig(Config.TLS_SECRET_VOLUME_NAME);
if (!isVolumeMountAlreadyExists(builder.buildVolumeMounts(), tlsSecretVolumeName)) {
String mountPath = resolveMountPointLocation(getConfig(Config.TLS_SECRET_VOLUME_MOUNT_POINT));
builder.addNewVolumeMount().withName(tlsSecretVolumeName)
.withMountPath(getConfig(Config.TLS_SECRET_VOLUME_MOUNT_POINT)).withReadOnly(true)
.withMountPath(mountPath).withReadOnly(true)
.endVolumeMount();
}

String jksVolumeName = getConfig(Config.JKS_VOLUME_NAME);
if (!isVolumeMountAlreadyExists(builder.buildVolumeMounts(), jksVolumeName)) {
String mountPath = resolveMountPointLocation(getConfig(Config.JKS_VOLUME_MOUNT_POINT));
builder.addNewVolumeMount().withName(jksVolumeName)
.withMountPath(getConfig(Config.JKS_VOLUME_MOUNT_POINT)).withReadOnly(true).endVolumeMount();
.withMountPath(mountPath).withReadOnly(true).endVolumeMount();
}
}

Expand All @@ -176,6 +179,10 @@ private boolean isVolumeMountAlreadyExists(List<VolumeMount> volumes, String vol
}
return false;
}

private String resolveMountPointLocation(String configValue) {
return String.format(configValue, JKubeAnnotations.getAnnotationPrefix(shouldUseOldJkubePrefix()));
}
});

builder.accept(new TypedVisitor<ServiceBuilder>() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ void getAnnotations_addedInKubernetesPlatformMode() {
Map<String, String> annotations;

// When
annotations = GitEnricher.getAnnotations(PlatformMode.kubernetes, GIT_REMOTE_URL, GIT_BRANCH, GIT_COMMIT_ID);
annotations = GitEnricher.getAnnotations(PlatformMode.kubernetes, GIT_REMOTE_URL, GIT_BRANCH, GIT_COMMIT_ID, true);

// Then
assertJkubeAnnotations(annotations);
Expand All @@ -46,7 +46,7 @@ void getAnnotations_addedInOpenShiftPlatformMode() {
Map<String, String> annotations;

// When
annotations = GitEnricher.getAnnotations(PlatformMode.openshift, GIT_REMOTE_URL, GIT_BRANCH, GIT_COMMIT_ID);
annotations = GitEnricher.getAnnotations(PlatformMode.openshift, GIT_REMOTE_URL, GIT_BRANCH, GIT_COMMIT_ID, true);

// Then
assertJkubeAnnotations(annotations);
Expand All @@ -60,7 +60,7 @@ void getAnnotations_addedWithAllNullValues() {
Map<String, String> annotations;

// When
annotations = GitEnricher.getAnnotations(PlatformMode.kubernetes, null, null, null);
annotations = GitEnricher.getAnnotations(PlatformMode.kubernetes, null, null, null, true);

// Then
assertThat(annotations).isEmpty();
Expand All @@ -72,15 +72,15 @@ void getAnnotations_addedWithNullCommitValues() {
Map<String, String> annotations;

// When
annotations = GitEnricher.getAnnotations(PlatformMode.kubernetes, GIT_REMOTE_URL, GIT_BRANCH, null);
annotations = GitEnricher.getAnnotations(PlatformMode.kubernetes, GIT_REMOTE_URL, GIT_BRANCH, null, true);

// Then
assertJkubeAnnotationsRemoteUrlAndBranch(annotations);
}

private void assertJkubeAnnotations(Map<String, String> annotations) {
assertJkubeAnnotationsRemoteUrlAndBranch(annotations);
assertThat(annotations).containsEntry(JKubeAnnotations.GIT_COMMIT.value(),GIT_COMMIT_ID);
assertThat(annotations).containsEntry(JKubeAnnotations.GIT_COMMIT.value(true), GIT_COMMIT_ID);
}

private void assertJkubeAnnotationsRemoteUrlAndBranch(Map<String, String> annotations) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ void mavenIssueManagementAll() {
Map<String, String> scmAnnotations = builder.buildFirstItem().getMetadata().getAnnotations();
assertThat(scmAnnotations).isNotNull()
.hasSize(2)
.containsEntry(JKubeAnnotations.ISSUE_SYSTEM.value(), "GitHub")
.containsEntry(JKubeAnnotations.ISSUE_TRACKER_URL.value(), "https://github.com/reactiverse/vertx-maven-plugin/issues/");
.containsEntry(JKubeAnnotations.ISSUE_SYSTEM.value(false), "GitHub")
.containsEntry(JKubeAnnotations.ISSUE_TRACKER_URL.value(false), "https://github.com/reactiverse/vertx-maven-plugin/issues/");
}

@DisplayName("maven issue management")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ void mavenScmAll() {
assertThat(scmAnnotations)
.isNotNull()
.hasSize(2)
.containsEntry("jkube.io/scm-tag", "HEAD")
.containsEntry("jkube.io/scm-url", "git://github.com/jkubeio/kubernetes-maven-plugin.git");
.containsEntry("jkube.eclipse.org/scm-tag", "HEAD")
.containsEntry("jkube.eclipse.org/scm-url", "git://github.com/jkubeio/kubernetes-maven-plugin.git");

}

Expand All @@ -78,8 +78,8 @@ void mavenScmOnlyDevConnection() {
Map<String, String> scmAnnotations = builder.buildFirstItem().getMetadata().getAnnotations();
assertThat(scmAnnotations).isNotNull()
.hasSize(1)
.containsEntry("jkube.io/scm-url", "git://github.com/jkubeio/kubernetes-maven-plugin.git")
.doesNotContainKey("jkube.io/scm-tag");
.containsEntry("jkube.eclipse.org/scm-url", "git://github.com/jkubeio/kubernetes-maven-plugin.git")
.doesNotContainKey("jkube.eclipse.org/scm-tag");
}

@Test
Expand All @@ -99,8 +99,8 @@ void mavenScmOnlyUrl() {
Map<String, String> scmAnnotations = builder.buildFirstItem().getMetadata().getAnnotations();
assertThat(scmAnnotations).isNotNull()
.hasSize(1)
.containsEntry("jkube.io/scm-url", "scm:git:git://github.com/jkubeio/kubernetes-maven-plugin.git")
.doesNotContainKey("jkube.io/scm-tag");
.containsEntry("jkube.eclipse.org/scm-url", "scm:git:git://github.com/jkubeio/kubernetes-maven-plugin.git")
.doesNotContainKey("jkube.eclipse.org/scm-tag");
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.eclipse.jkube.kit.common.RegistryServerConfiguration;
import org.eclipse.jkube.kit.common.util.KubernetesHelper;
import org.eclipse.jkube.kit.common.util.ResourceUtil;
import org.eclipse.jkube.kit.config.resource.JKubeAnnotations;

import java.io.File;
import java.io.FilenameFilter;
Expand All @@ -32,6 +33,7 @@
import java.util.Collections;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.function.Supplier;
Expand Down Expand Up @@ -184,7 +186,8 @@ static String findIconURL(File manifest) {
throw new IllegalStateException("Failed to load kubernetes YAML " + manifest + ". " + e, e);
}
if (dto instanceof HasMetadata) {
answer = KubernetesHelper.getOrCreateAnnotations((HasMetadata) dto).get("jkube.io/iconUrl");
Map<String, String> annotations = KubernetesHelper.getOrCreateAnnotations((HasMetadata) dto);
answer = getJKubeIconUrlFromAnnotations(annotations);
}
answer = extractIconUrlAnnotationFromKubernetesList(answer, dto);
}
Expand Down Expand Up @@ -270,7 +273,8 @@ private static String extractIconUrlAnnotationFromKubernetesList(String answer,
List<HasMetadata> items = list.getItems();
if (items != null) {
for (HasMetadata item : items) {
answer = KubernetesHelper.getOrCreateAnnotations(item).get("jkube.io/iconUrl");
Map<String, String> annotations = KubernetesHelper.getOrCreateAnnotations(item);
answer = getJKubeIconUrlFromAnnotations(annotations);
if (StringUtils.isNotBlank(answer)) {
break;
}
Expand All @@ -280,4 +284,13 @@ private static String extractIconUrlAnnotationFromKubernetesList(String answer,
return answer;
}

private static String getJKubeIconUrlFromAnnotations(Map<String, String> annotations) {
if (annotations.containsKey(JKubeAnnotations.ICON_URL.value(true))) {
return annotations.get(JKubeAnnotations.ICON_URL.value(true));
}
if (annotations.containsKey(JKubeAnnotations.ICON_URL.value(false))) {
return annotations.get(JKubeAnnotations.ICON_URL.value(false));
}
return null;
}
}

0 comments on commit 3be8092

Please sign in to comment.