EnvironmentVariable invalid keys in method public IEnvVarSource getValueFrom

[tszielin]

In the class com.openshift.internal.restclient.model.EnvironmentVariable the method public IEnvVarSource getValueFrom() always return null.

Probably the problem is cause by first condition which checks getNode().hasDefined("fieldRef"). The result of getNode().keys() is ["name', "valueFrom"] instead of "valueFrom" . Changed method parameters like getNode().hasDefined("valueFrom", "fieldRef") solves the condition.

Also the keys in next should be changed by adding "valueFrom." like return asString(getNode(), getPropertyKeys(), "valueFrom.fieldRef.apiVersion");

[adietish]

Hi @tszielin

thx for reporting, I'd need a concrete example to reproduce this though. Your concerns about the code seem valid but to be very sure things work as expected I'd love to have some steps that I can follow. Can you please provide these?

[adietish]

Hi @tszielin

I found a json that could allow me to test what you found:

{
        "name": "ENV_FIELD_REF",
                "valueFrom": { "fieldRef": {
                        "fieldPath": "metadata.namespace"
                }}
},

Is this matching what you're using when you found the issue?

[adietish]

I could replicate your issue. Here's a PR that should fix it: https://github.com/openshift/openshift-restclient-java/pull/445 I removed IObjectFieldSelector.getApiVersion() since I couldn't find any example where such property was still exposed. Could you please test it and report back?

[adietish]

@tszielin the fix is available in openshift-restclient-java 9.0.0.Final that I just published

[tszielin]

Hi @adietish Please take a look to code `import static com.openshift.internal.util.JBossDmrExtentions.asString;

import java.util.List; import java.util.Optional; import org.apache.commons.lang.StringUtils; import org.jboss.dmr.ModelNode;

import com.openshift.internal.restclient.model.EnvironmentVariable; import com.openshift.restclient.ClientBuilder; import com.openshift.restclient.IClient; import com.openshift.restclient.OpenShiftException; import com.openshift.restclient.ResourceKind; import com.openshift.restclient.model.IConfigMapKeySelector; import com.openshift.restclient.model.IDeploymentConfig; import com.openshift.restclient.model.IEnvironmentVariable; import com.openshift.restclient.model.IObjectFieldSelector; import com.openshift.restclient.model.IResource; import com.openshift.restclient.model.ISecretKeySelector; import lombok.extern.slf4j.Slf4j;

@Slf4j public class OpenShiftClient { private IClient client;

class EnvVariable extends EnvironmentVariable {
    public EnvVariable(ModelNode node) {
      super(node, null);
    }

    @Override
    public IEnvVarSource getValueFrom() {
        if (getNode().hasDefined("valueFrom", "fieldRef")) {
            return new IObjectFieldSelector() {
                @Override
                public String getApiVersion() {
                    return asString(getNode(), getPropertyKeys(), "valueFrom.fieldRef.apiVersion");
                }

                @Override
                public String getFieldPath() {
                    return asString(getNode(), getPropertyKeys(), "valueFrom.fieldRef.fieldPath");
                }

            };
        } else if (getNode().hasDefined("valueFrom", "configMapKeyRef")) {
            return new IConfigMapKeySelector() {

                @Override
                public String getName() {
                    return asString(getNode(), getPropertyKeys(), "valueFrom.configMapKeyRef.name");
                }

                @Override
                public String getKey() {
                    return asString(getNode(), getPropertyKeys(), "valueFrom.configMapKeyRef.key");
                }
            };

        } else if (getNode().hasDefined("valueFrom", "secretKeyRef")) {
            return new ISecretKeySelector() {

                @Override
                public String getName() {
                    return asString(getNode(), getPropertyKeys(), "valueFrom.secretKeyRef.name");
                }

                @Override
                public String getKey() {
                    return asString(getNode(), getPropertyKeys(), "valueFrom.secretKeyRef.key");
                }
            };
        }
        return null;
    }
}

public OpenShiftClient(String url, String userName, String password) {
    try {
        this.client = new ClientBuilder(url)
                .withUserName(userName).withPassword(password).build();
        log.info("OpenShift {}.{} (OpenShift API {}, Kubernetes {}) responds with status: {}",
                this.client.getOpenshiftMasterVersion(), this.client.getOpenShiftMajorVersion(),
                this.client.getOpenShiftAPIVersion(), this.client.getKubernetesMasterVersion(),
                this.client.getServerReadyStatus());
    } catch (Exception ex) {
        log.error(ex.getMessage());
        throw new IllegalArgumentException(ex.getMessage());
    }
}

public void process(String namespace) {
    List<IResource> resources = getResources(namespace);

    if (resources == null || resources.isEmpty()) {
        log.error("Cannot read resources from OpenShift");
        return;
    }

    IDeploymentConfig dc = resources.stream()
        .filter(r -> r instanceof IDeploymentConfig)
        .map(r -> (IDeploymentConfig)r)
        .findAny().get();
    if (dc != null) {
        dc.getEnvironmentVariables().forEach(v -> {
            log.info("Current implementatation");
            processStandard(v);
            log.info("Patched implementatation");
            processPatched(v);
        });
    }
}

private void processStandard(IEnvironmentVariable variable) {
    log.info("variable.toJson(): " + variable.toJson());
    log.info("variable.getValueFrom(): " + String.valueOf(variable.getValueFrom()));
}

private void processPatched(IEnvironmentVariable v) {
    EnvVariable patchedVariable = new EnvVariable(ModelNode.fromJSONString(v.toJson()));
    log.info("patchedVariable.toJson(): " + patchedVariable.toJson());
    log.info("patchedVariable.getValueFrom(): " + String.valueOf(patchedVariable.getValueFrom()));
    log.info("patchedVariable.getValueFrom() instanceof IObjectFieldSelector: " + (patchedVariable.getValueFrom() instanceof IObjectFieldSelector));
    log.info("patchedVariable.getValueFrom() instanceof IConfigMapKeySelector: " + (patchedVariable.getValueFrom() instanceof IConfigMapKeySelector));
    log.info("patchedVariable.getValueFrom() instanceof ISecretKeySelector: " + (patchedVariable.getValueFrom() instanceof ISecretKeySelector));
}

private List<IResource> getResources(String namespace) {
    try {
        return client != null ? client.list(ResourceKind.DEPLOYMENT_CONFIG, namespace) : List.of();
    } catch (OpenShiftException ex) {
        log.error(Optional.ofNullable(ex.getMessage())
                .filter(StringUtils::isNotBlank)
                .orElse(Optional.ofNullable(ex.getStatus())
                        .map(status -> String.format("%s [%d].", status.getMessage(), status.getCode()))
                        .orElse("Unexpected error.")));
        return List.of();
    }
}

public static void main(String[] args) {
    new OpenShiftClient(args[0], args[1], args[2]).process(args[3]);
}

}`

this produces 2020-04-15 20:54:58.283 INFO 49166 --- [ restartedMain] tszielin.OpenShiftClient : OpenShift v3.11.154.3 (OpenShift API v1, Kubernetes v1.11.0+d4cacc0) responds with status: ok

2020-04-15 20:54:59.416 INFO 49166 --- [ restartedMain] tszielin.OpenShiftClient : Current implementatation

2020-04-15 20:54:59.417 INFO 49166 --- [ restartedMain] tszielin.OpenShiftClient : variable.toJson(): { "name" : "NODE_NAME", "valueFrom" : {"fieldRef" : { "apiVersion" : "v1", "fieldPath" : "metadata.name" }} }

2020-04-15 20:54:59.417 INFO 49166 --- [ restartedMain] tszielin.OpenShiftClient : variable.getValueFrom(): null

2020-04-15 20:54:59.417 INFO 49166 --- [ restartedMain] tszielin.OpenShiftClient : Patched implementatation

2020-04-15 20:54:59.417 INFO 49166 --- [ restartedMain] tszielin.OpenShiftClient : patchedVariable.toJson(): { "name" : "NODE_NAME", "valueFrom" : {"fieldRef" : { "apiVersion" : "v1", "fieldPath" : "metadata.name" }} }

2020-04-15 20:54:59.418 INFO 49166 --- [ restartedMain] tszielin.OpenShiftClient : patchedVariable.getValueFrom(): tszielin.OpenShiftClient$EnvVariable$1@5f123cb3

2020-04-15 20:54:59.418 INFO 49166 --- [ restartedMain] tszielin.OpenShiftClient : patchedVariable.getValueFrom() instanceof IObjectFieldSelector: true

2020-04-15 20:54:59.418 INFO 49166 --- [ restartedMain] tszielin.OpenShiftClient : patchedVariable.getValueFrom() instanceof IConfigMapKeySelector: false

2020-04-15 20:54:59.419 INFO 49166 --- [ restartedMain] tszielin.OpenShiftClient : patchedVariable.getValueFrom() instanceof ISecretKeySelector: false

2020-04-15 20:54:59.419 INFO 49166 --- [ restartedMain] tszielin.OpenShiftClient : Current implementatation

2020-04-15 20:54:59.419 INFO 49166 --- [ restartedMain] tszielin.OpenShiftClient : variable.toJson(): { "name" : "OPENSHIFT_CONSOLE_URL", "value" : "https://console:8443" }

2020-04-15 20:54:59.419 INFO 49166 --- [ restartedMain] tszielin.OpenShiftClient : variable.getValueFrom(): null

2020-04-15 20:54:59.419 INFO 49166 --- [ restartedMain] tszielin.OpenShiftClient : Patched implementatation

2020-04-15 20:54:59.419 INFO 49166 --- [ restartedMain] tszielin.OpenShiftClient : patchedVariable.toJson(): { "name" : "OPENSHIFT_CONSOLE_URL", "value" : "https://console:8443" }

2020-04-15 20:54:59.419 INFO 49166 --- [ restartedMain] tszielin.OpenShiftClient : patchedVariable.getValueFrom(): null

2020-04-15 20:54:59.419 INFO 49166 --- [ restartedMain] tszielin.OpenShiftClient : patchedVariable.getValueFrom() instanceof IObjectFieldSelector: false

2020-04-15 20:54:59.419 INFO 49166 --- [ restartedMain] tszielin.OpenShiftClient : patchedVariable.getValueFrom() instanceof IConfigMapKeySelector: false

2020-04-15 20:54:59.419 INFO 49166 --- [ restartedMain] tszielin.OpenShiftClient : patchedVariable.getValueFrom() instanceof ISecretKeySelector: false

2020-04-15 20:54:59.419 INFO 49166 --- [ restartedMain] tszielin.OpenShiftClient : Current implementatation

2020-04-15 20:54:59.419 INFO 49166 --- [ restartedMain] tszielin.OpenShiftClient : variable.toJson(): { "name" : "OPENSHIFT_TOKEN", "valueFrom" : {"secretKeyRef" : { "name" : "credentials", "key" : "password" }} }

2020-04-15 20:54:59.419 INFO 49166 --- [ restartedMain] tszielin.OpenShiftClient : variable.getValueFrom(): null

2020-04-15 20:54:59.419 INFO 49166 --- [ restartedMain] tszielin.OpenShiftClient : Patched implementatation

2020-04-15 20:54:59.419 INFO 49166 --- [ restartedMain] tszielin.OpenShiftClient : patchedVariable.toJson(): { "name" : "OPENSHIFT_TOKEN", "valueFrom" : {"secretKeyRef" : { "name" : "credentials", "key" : "password" }} }

2020-04-15 20:54:59.420 INFO 49166 --- [ restartedMain] tszielin.OpenShiftClient : patchedVariable.getValueFrom(): tszielin.OpenShiftClient$EnvVariable$3@3c658bbd

2020-04-15 20:54:59.420 INFO 49166 --- [ restartedMain] tszielin.OpenShiftClient : patchedVariable.getValueFrom() instanceof IObjectFieldSelector: false

2020-04-15 20:54:59.420 INFO 49166 --- [ restartedMain] tszielin.OpenShiftClient : patchedVariable.getValueFrom() instanceof IConfigMapKeySelector: false

2020-04-15 20:54:59.420 INFO 49166 --- [ restartedMain] tszielin.OpenShiftClient : patchedVariable.getValueFrom() instanceof ISecretKeySelector: true

As we can observe 'standard' implementation always returns null when calling v.getValueFrom(). EnvVariable implementation reads proper (I hope) valueFrom if exists.

[adietish]

Hi @tszielin

The code that you pasted above is using outdated code that existed before I merged #445:

    public IEnvVarSource getValueFrom() {
        if (getNode().hasDefined("valueFrom", "fieldRef")) {
            return new IObjectFieldSelector() {
                @Override
                public String getApiVersion() {
                    return asString(getNode(), getPropertyKeys(), "valueFrom.fieldRef.apiVersion");
                }

                @Override
                public String getFieldPath() {
                    return asString(getNode(), getPropertyKeys(), "valueFrom.fieldRef.fieldPath");
                }

            };
        } else if (getNode().hasDefined("valueFrom", "configMapKeyRef")) {
            return new IConfigMapKeySelector() {

                @Override
                public String getName() {
                    return asString(getNode(), getPropertyKeys(), "valueFrom.configMapKeyRef.name");
                }

                @Override
                public String getKey() {
                    return asString(getNode(), getPropertyKeys(), "valueFrom.configMapKeyRef.key");
                }
            };

        } else if (getNode().hasDefined("valueFrom", "secretKeyRef")) {
            return new ISecretKeySelector() {

                @Override
                public String getName() {
                    return asString(getNode(), getPropertyKeys(), "valueFrom.secretKeyRef.name");
                }

                @Override
                public String getKey() {
                    return asString(getNode(), getPropertyKeys(), "valueFrom.secretKeyRef.key");
                }
            };
        }
        return null;
    }

The new code looks as follows:

public IEnvVarSource getValueFrom() {
        ModelNode valueFrom = getNode().get(PROP_VALUE_FROM);
        if (valueFrom == null) {
            return null;
        }
        if (valueFrom.hasDefined(PROP_FIELD_REF)) {
            return createObjectFieldSelector(valueFrom);
        } else if (valueFrom.hasDefined(PROP_CONFIG_MAP_KEY_REF)) {
            return createConfigMapKeySelector(valueFrom);
        } else if (valueFrom.hasDefined(PROP_SECRET_KEY_REF)) {
            return createSecretKeySelector(valueFrom);
        }
        return null;
}

With the corrected code EnvironmentVariable should work as expected and you should not have to subclass it. Doesn't it for you?

[tszielin]

Perfect.