Skip to content

chore: remove unused EnvVarUtils.validateEnvVars methods #32781

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.

Sign up for GitHub

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

Open
detail-app wants to merge 1 commit into
base: main
Choose a base branch
from
+0 −111
Open

chore: remove unused EnvVarUtils.validateEnvVars methods #32781

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
46 changes: 0 additions & 46 deletions openmetadata-k8s-operator/src/main/java/org/openmetadata/operator/util/EnvVarUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,50 +95,4 @@ private static boolean hasEmptyValueFrom(EnvVar envVar) {
&& envVar.getValueFrom().getFieldRef() == null
&& envVar.getValueFrom().getResourceFieldRef() == null;
}

/**
* Validate that environment variables are properly configured.
*
* @param envVars List of environment variables to validate
* @return true if all variables are valid
*/
public static boolean validateEnvVars(List<EnvVar> envVars) {
if (envVars == null) {
return true;
}

for (EnvVar envVar : envVars) {
if (!validateEnvVar(envVar)) {
return false;
}
}
return true;
}

/**
* Validate a single environment variable.
*
* @param envVar Environment variable to validate
* @return true if variable is valid
*/
private static boolean validateEnvVar(EnvVar envVar) {
if (envVar == null || envVar.getName() == null || envVar.getName().isEmpty()) {
LOG.error("Invalid env var: missing name");
return false;
}

// Must have either value or valueFrom, not both
if (envVar.getValue() != null && envVar.getValueFrom() != null) {
LOG.error("Invalid env var {}: cannot have both value and valueFrom", envVar.getName());
return false;
}

// If valueFrom exists, it must have at least one valid reference
if (envVar.getValueFrom() != null && hasEmptyValueFrom(envVar)) {
LOG.error("Invalid env var {}: empty valueFrom", envVar.getName());
return false;
}

return true;
}
}
65 changes: 0 additions & 65 deletions openmetadata-k8s-operator/src/test/java/org/openmetadata/operator/util/EnvVarUtilsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,65 +159,6 @@ void testSanitizeEnvVarsWithMixedTypes() {
assertNotNull(result.get(2).getValueFrom().getConfigMapKeyRef());
}

@Test
void testValidateEnvVarsWithValidVars() {
List<EnvVar> envVars =
Arrays.asList(
new EnvVarBuilder().withName("VAR1").withValue("value1").build(),
new EnvVarBuilder()
.withName("VAR2")
.withValueFrom(
new EnvVarSourceBuilder()
.withConfigMapKeyRef(
new ConfigMapKeySelectorBuilder()
.withName("config")
.withKey("key")
.build())
.build())
.build());

assertTrue(EnvVarUtils.validateEnvVars(envVars));
}

@Test
void testValidateEnvVarsWithEmptyValueFrom() {
List<EnvVar> envVars =
List.of(
new EnvVarBuilder()
.withName("INVALID")
.withValueFrom(new EnvVarSource()) // Empty valueFrom is invalid
.build());

assertFalse(EnvVarUtils.validateEnvVars(envVars));
}

@Test
void testValidateEnvVarsWithBothValueAndValueFrom() {
List<EnvVar> envVars =
List.of(
new EnvVarBuilder()
.withName("INVALID")
.withValue("value")
.withValueFrom(
new EnvVarSourceBuilder()
.withConfigMapKeyRef(
new ConfigMapKeySelectorBuilder()
.withName("config")
.withKey("key")
.build())
.build())
.build());

assertFalse(EnvVarUtils.validateEnvVars(envVars));
}

@Test
void testValidateEnvVarsWithMissingName() {
List<EnvVar> envVars = List.of(new EnvVarBuilder().withValue("value").build());

assertFalse(EnvVarUtils.validateEnvVars(envVars));
}

@Test
void testRealWorldScenarioFromFailedCronOMJob() {
// This recreates the exact scenario from the failed CronOMJob
Expand All @@ -242,15 +183,9 @@ void testRealWorldScenarioFromFailedCronOMJob() {
.withValue("openmetadata-pipelines-test")
.build());

// Before sanitization - validation should fail
assertFalse(EnvVarUtils.validateEnvVars(envVars), "Original env vars should be invalid");

// After sanitization
List<EnvVar> sanitized = EnvVarUtils.sanitizeEnvVars(envVars);

// Should pass validation now
assertTrue(EnvVarUtils.validateEnvVars(sanitized), "Sanitized env vars should be valid");

// Check that all vars are present
assertEquals(6, sanitized.size());

Expand Down
Loading