diff options
20 files changed, 331 insertions, 246 deletions
diff --git a/client/go/internal/admin/jvm/env.go b/client/go/internal/admin/jvm/env.go index 7b1ce97a40a..1cbdb46648f 100644 --- a/client/go/internal/admin/jvm/env.go +++ b/client/go/internal/admin/jvm/env.go @@ -5,10 +5,12 @@ package jvm import ( "fmt" + "strings" "github.com/vespa-engine/vespa/client/go/internal/admin/defaults" "github.com/vespa-engine/vespa/client/go/internal/admin/envvars" "github.com/vespa-engine/vespa/client/go/internal/admin/prog" + "github.com/vespa-engine/vespa/client/go/internal/admin/trace" "github.com/vespa-engine/vespa/client/go/internal/util" ) @@ -29,8 +31,19 @@ func (opts *Options) exportEnvSettings(ps *prog.Spec) { ps.Setenv(envvars.LD_LIBRARY_PATH, dlp) ps.Setenv(envvars.MALLOC_ARENA_MAX, "1") if preload := ps.Getenv(envvars.PRELOAD); preload != "" { - ps.Setenv(envvars.JAVAVM_LD_PRELOAD, preload) - ps.Setenv(envvars.LD_PRELOAD, preload) + checked := []string{} + for _, fileName := range strings.Split(preload, ":") { + if util.PathExists(fileName) { + checked = append(checked, fileName) + } else { + trace.Info("File in PRELOAD missing, skipped:", fileName) + } + } + if len(checked) > 0 { + preload := strings.Join(checked, ":") + ps.Setenv(envvars.JAVAVM_LD_PRELOAD, preload) + ps.Setenv(envvars.LD_PRELOAD, preload) + } } util.OptionallyReduceTimerFrequency() c.exportExtraEnv(ps) diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/DispatchTuningTest.java b/config-model/src/test/java/com/yahoo/vespa/model/content/DispatchTuningTest.java index af547965749..15fba6a7dc9 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/content/DispatchTuningTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/content/DispatchTuningTest.java @@ -15,13 +15,13 @@ public class DispatchTuningTest { void requireThatAccessorWork() { DispatchTuning dispatch = new DispatchTuning.Builder() .setMaxHitsPerPartition(69) - .setDispatchPolicy("round-robin") + .setDispatchPolicy("best-of-random-2") .setMinActiveDocsCoverage(12.5) .setTopKProbability(18.3) .build(); assertEquals(69, dispatch.getMaxHitsPerPartition().intValue()); assertEquals(12.5, dispatch.getMinActiveDocsCoverage(), 0.0); - assertEquals(DispatchTuning.DispatchPolicy.ROUNDROBIN, dispatch.getDispatchPolicy()); + assertEquals(DispatchTuning.DispatchPolicy.BEST_OF_RANDOM_2, dispatch.getDispatchPolicy()); assertEquals(18.3, dispatch.getTopkProbability(), 0.0); } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/IndexedHierarchicDistributionTest.java b/config-model/src/test/java/com/yahoo/vespa/model/content/IndexedHierarchicDistributionTest.java index 8a47eb030f3..ab147f22e8b 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/content/IndexedHierarchicDistributionTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/content/IndexedHierarchicDistributionTest.java @@ -144,30 +144,6 @@ public class IndexedHierarchicDistributionTest { "</tuning>"); } - private String getRandomDispatchXml() { - return joinLines("<tuning>", - " <dispatch>", - " <dispatch-policy>random</dispatch-policy>", - " </dispatch>", - "</tuning>"); - } - - private ContentCluster getOddGroupsCluster() throws Exception { - String groupXml = joinLines(" <group>", - " <distribution partitions='2|*'/>", - " <group distribution-key='0' name='group0'>", - " <node distribution-key='0' hostalias='mockhost'/>", - " <node distribution-key='1' hostalias='mockhost'/>", - " </group>", - " <group distribution-key='1' name='group1'>", - " <node distribution-key='3' hostalias='mockhost'/>", - " <node distribution-key='4' hostalias='mockhost'/>", - " <node distribution-key='5' hostalias='mockhost'/>", - " </group>", - " </group>", ""); - return createCluster(createClusterXml(groupXml, Optional.of(getRandomDispatchXml()), 4, 4)); - } - @Test void requireThatWeMustHaveOnlyOneGroupLevel() { try { diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/cluster/ClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/content/cluster/ClusterTest.java index a6ea6cb8132..0800f26d6e8 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/content/cluster/ClusterTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/content/cluster/ClusterTest.java @@ -72,7 +72,7 @@ public class ClusterTest { "", joinLines( "<max-hits-per-partition>77</max-hits-per-partition>", - "<dispatch-policy>round-robin</dispatch-policy>", + "<dispatch-policy>best-of-random-2</dispatch-policy>", "<min-active-docs-coverage>93</min-active-docs-coverage>", "<top-k-probability>0.777</top-k-probability>"), false); @@ -81,7 +81,7 @@ public class ClusterTest { DispatchConfig config = new DispatchConfig(builder); assertEquals(3, config.redundancy()); assertEquals(93.0, config.minActivedocsPercentage(), DELTA); - assertEquals(DispatchConfig.DistributionPolicy.ROUNDROBIN, config.distributionPolicy()); + assertEquals(DispatchConfig.DistributionPolicy.BEST_OF_RANDOM_2, config.distributionPolicy()); assertEquals(77, config.maxHitsPerNode()); assertEquals(0.777, config.topKProbability(), DELTA); } diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/SystemName.java b/config-provisioning/src/main/java/com/yahoo/config/provision/SystemName.java index e2349f6f63f..c6eecf1e705 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/SystemName.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/SystemName.java @@ -75,4 +75,6 @@ public enum SystemName { return Stream.of(values()).filter(predicate).collect(Collectors.toUnmodifiableSet()); } + public static Set<SystemName> hostedVespa() { return EnumSet.of(main, cd, Public, PublicCd); } + } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/FlagValidationException.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/FlagValidationException.java new file mode 100644 index 00000000000..00c88102819 --- /dev/null +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/FlagValidationException.java @@ -0,0 +1,11 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.api.systemflags.v1; + +/** + * @author hakonhall + */ +public class FlagValidationException extends RuntimeException { + public FlagValidationException(String message) { + super(message); + } +} diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/FlagsTarget.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/FlagsTarget.java index bad53620c81..7403f0a1b01 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/FlagsTarget.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/FlagsTarget.java @@ -80,6 +80,54 @@ public interface FlagsTarget { static String zoneFile(SystemName system, ZoneId zone) { return jsonFile(system.value() + "." + zone.environment().value() + "." + zone.region().value()); } static String controllerFile(SystemName system) { return jsonFile(system.value() + ".controller"); } + /** Return true if the filename applies to the system. Throws on invalid filename format. */ + static boolean filenameForSystem(String filename, SystemName system) throws FlagValidationException { + if (filename.equals(defaultFile())) return true; + + String[] parts = filename.split("\\.", -1); + if (parts.length < 2) throw new FlagValidationException("Invalid flag filename: " + filename); + + if (!parts[parts.length - 1].equals("json")) throw new FlagValidationException("Invalid flag filename: " + filename); + + SystemName systemFromFile; + try { + systemFromFile = SystemName.from(parts[0]); + } catch (IllegalArgumentException e) { + throw new FlagValidationException("First part of flag filename is neither 'default' nor a valid system: " + filename); + } + if (!SystemName.hostedVespa().contains(systemFromFile)) + throw new FlagValidationException("Unknown system in flag filename: " + filename); + if (!systemFromFile.equals(system)) return false; + + if (parts.length == 2) return true; // systemFile + + if (parts.length == 3) { + if (parts[1].equals("controller")) return true; // controllerFile + try { + Environment.from(parts[1]); + } catch (IllegalArgumentException e) { + throw new FlagValidationException("Invalid environment in flag filename: " + filename); + } + return true; // environmentFile + } + + if (parts.length == 4) { + try { + Environment.from(parts[1]); + } catch (IllegalArgumentException e) { + throw new FlagValidationException("Invalid environment in flag filename: " + filename); + } + try { + RegionName.from(parts[2]); + } catch (IllegalArgumentException e) { + throw new FlagValidationException("Invalid region in flag filename: " + filename); + } + return true; // zoneFile + } + + throw new FlagValidationException("Invalid flag filename: " + filename); + } + /** Partially resolve inter-zone dimensions, except those dimensions defined by the flag for a controller zone. */ static FlagData partialResolve(FlagData data, SystemName system, CloudName cloud, ZoneId virtualZoneId) { Set<FetchVector.Dimension> flagDimensions = diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchive.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchive.java index 8ca4c37a85a..9426952f57e 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchive.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchive.java @@ -23,6 +23,7 @@ import com.yahoo.vespa.flags.json.FlagData; import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneRegistry; import java.io.BufferedInputStream; +import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; @@ -32,7 +33,6 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; import java.util.ArrayList; -import java.util.Arrays; import java.util.List; import java.util.Map; import java.util.Objects; @@ -74,7 +74,7 @@ public class SystemFlagsDataArchive { this.files = files; } - public static SystemFlagsDataArchive fromZip(InputStream rawIn) { + public static SystemFlagsDataArchive fromZip(InputStream rawIn, ZoneRegistry zoneRegistry) { Builder builder = new Builder(); try (ZipInputStream zipIn = new ZipInputStream(new BufferedInputStream(rawIn))) { ZipEntry entry; @@ -83,7 +83,7 @@ public class SystemFlagsDataArchive { if (!entry.isDirectory() && name.startsWith("flags/")) { Path filePath = Paths.get(name); String rawData = new String(zipIn.readAllBytes(), StandardCharsets.UTF_8); - addFile(builder, rawData, filePath, Set.of(), null); + addFile(builder, rawData, filePath, zoneRegistry, true); } } return builder.build(); @@ -92,27 +92,19 @@ public class SystemFlagsDataArchive { } } - public static SystemFlagsDataArchive fromDirectoryAndSystem(Path directory, ZoneRegistry systemDefinition) { - return fromDirectory(directory, systemDefinition); - } - - public static SystemFlagsDataArchive fromDirectory(Path directory) { return fromDirectory(directory, null); } - - private static SystemFlagsDataArchive fromDirectory(Path directory, ZoneRegistry systemDefinition) { - Set<String> filenamesForSystem = getFilenamesForSystem(systemDefinition); + public static SystemFlagsDataArchive fromDirectory(Path directory, ZoneRegistry zoneRegistry, boolean forceAddFiles) { Path root = directory.toAbsolutePath(); Path flagsDirectory = directory.resolve("flags"); if (!Files.isDirectory(flagsDirectory)) { - throw new IllegalArgumentException("Sub-directory 'flags' does not exist: " + flagsDirectory); + throw new FlagValidationException("Sub-directory 'flags' does not exist: " + flagsDirectory); } - try (Stream<Path> directoryStream = Files.walk(root)) { + try (Stream<Path> directoryStream = Files.walk(flagsDirectory)) { Builder builder = new Builder(); - directoryStream.forEach(absolutePath -> { - Path relativePath = root.relativize(absolutePath); - if (!Files.isDirectory(absolutePath) && - relativePath.startsWith("flags")) { - String rawData = uncheck(() -> Files.readString(absolutePath, StandardCharsets.UTF_8)); - addFile(builder, rawData, relativePath, filenamesForSystem, systemDefinition); + directoryStream.forEach(path -> { + Path relativePath = root.relativize(path.toAbsolutePath()); + if (Files.isRegularFile(path)) { + String rawData = uncheck(() -> Files.readString(path, StandardCharsets.UTF_8)); + addFile(builder, rawData, relativePath, zoneRegistry, forceAddFiles); } }); return builder.build(); @@ -121,6 +113,14 @@ public class SystemFlagsDataArchive { } } + public byte[] toZipBytes() { + try (ByteArrayOutputStream out = new ByteArrayOutputStream()) { + toZip(out); + return out.toByteArray(); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } public void toZip(OutputStream out) { ZipOutputStream zipOut = new ZipOutputStream(out); @@ -153,67 +153,47 @@ public class SystemFlagsDataArchive { return targetData; } - public void validateAllFilesAreForTargets(SystemName currentSystem, Set<FlagsTarget> targets) throws IllegalArgumentException { + public void validateAllFilesAreForTargets(Set<FlagsTarget> targets) throws FlagValidationException { Set<String> validFiles = targets.stream() - .flatMap(target -> target.flagDataFilesPrioritized().stream()) - .collect(Collectors.toSet()); - Set<SystemName> otherSystems = Arrays.stream(SystemName.values()) - .filter(systemName -> systemName != currentSystem) - .collect(Collectors.toSet()); - files.forEach((flagId, fileMap) -> { - for (String filename : fileMap.keySet()) { - boolean isFileForOtherSystem = otherSystems.stream() - .anyMatch(system -> filename.startsWith(system.value() + ".")); - boolean isFileForCurrentSystem = validFiles.contains(filename); - if (!isFileForOtherSystem && !isFileForCurrentSystem) { - throw new IllegalArgumentException("Unknown flag file: " + toFilePath(flagId, filename)); - } + .flatMap(target -> target.flagDataFilesPrioritized().stream()) + .collect(Collectors.toSet()); + files.forEach((flagId, fileMap) -> fileMap.keySet().forEach(filename -> { + if (!validFiles.contains(filename)) { + throw new FlagValidationException("Unknown flag file: " + toFilePath(flagId, filename)); } - }); + })); } - private static Set<String> getFilenamesForSystem(ZoneRegistry systemDefinition) { - if (systemDefinition == null) return Set.of(); - return FlagsTarget.getAllTargetsInSystem(systemDefinition, false).stream() - .flatMap(target -> target.flagDataFilesPrioritized().stream()) - .collect(Collectors.toSet()); + boolean hasFlagData(FlagId flagId, String filename) { + return files.getOrDefault(flagId, Map.of()).containsKey(filename); } - private static void addFile(Builder builder, String rawData, Path filePath, Set<String> filenamesForSystem, - ZoneRegistry systemDefinition) { + private static void addFile(Builder builder, String rawData, Path filePath, ZoneRegistry zoneRegistry, boolean force) { String filename = filePath.getFileName().toString(); - if (filename.startsWith(".")) { + + if (filename.startsWith(".")) return; // Ignore files starting with '.' - } - if (!filenamesForSystem.isEmpty() && !filenamesForSystem.contains(filename)) { - if (systemDefinition != null && filename.startsWith(systemDefinition.system().value() + '.')) { - throw new IllegalArgumentException(String.format( - "Environment or zone in filename '%s' does not exist", filename)); - } - return; // Ignore files irrelevant for system - } - if (!filename.endsWith(".json")) { - throw new IllegalArgumentException(String.format("Only JSON files are allowed in 'flags/' directory (found '%s')", filePath.toString())); - } + + if (!force && !FlagsTarget.filenameForSystem(filename, zoneRegistry.system())) + return; // Ignore files for other systems + FlagId directoryDeducedFlagId = new FlagId(filePath.getName(filePath.getNameCount()-2).toString()); FlagData flagData; if (rawData.isBlank()) { flagData = new FlagData(directoryDeducedFlagId); } else { - Set<ZoneId> zones = systemDefinition == null ? - Set.of() : - systemDefinition.zones().all().zones().stream().map(ZoneApi::getVirtualId).collect(Collectors.toSet()); + Set<ZoneId> zones = force ? zoneRegistry.zones().all().zones().stream().map(ZoneApi::getVirtualId).collect(Collectors.toSet()) + : Set.of(); String normalizedRawData = normalizeJson(rawData, zones); flagData = FlagData.deserialize(normalizedRawData); if (!directoryDeducedFlagId.equals(flagData.id())) { - throw new IllegalArgumentException( - String.format("Flag data file with flag id '%s' in directory for '%s'", - flagData.id(), directoryDeducedFlagId.toString())); + throw new FlagValidationException("Flag data file with flag id '%s' in directory for '%s'" + .formatted(flagData.id(), directoryDeducedFlagId.toString())); } String serializedData = flagData.serializeToJson(); if (!JSON.equals(serializedData, normalizedRawData)) { - throw new IllegalArgumentException(""" + throw new FlagValidationException(""" %s contains unknown non-comment fields or rules with null values: after removing any comment fields the JSON is: %s but deserializing this ended up with: @@ -225,9 +205,8 @@ public class SystemFlagsDataArchive { } if (builder.hasFile(filename, flagData)) { - throw new IllegalArgumentException( - String.format("Flag data file in '%s' contains redundant flag data for id '%s' already set in another directory!", - filePath, flagData.id())); + throw new FlagValidationException("Flag data file in '%s' contains redundant flag data for id '%s' already set in another directory!" + .formatted(filePath, flagData.id())); } builder.addFile(filename, flagData); @@ -247,13 +226,13 @@ public class SystemFlagsDataArchive { FetchVector.Dimension dimension = DimensionHelper .fromWire(condition.get("dimension") .asString() - .orElseThrow(() -> new IllegalArgumentException("Invalid dimension in condition: " + condition))); + .orElseThrow(() -> new FlagValidationException("Invalid dimension in condition: " + condition))); switch (dimension) { case APPLICATION_ID -> validateStringValues(condition, ApplicationId::fromSerializedForm); case CONSOLE_USER_EMAIL -> validateStringValues(condition, email -> {}); case CLOUD -> validateStringValues(condition, cloud -> { if (!Set.of(YAHOO, AWS, GCP).contains(CloudName.from(cloud))) - throw new IllegalArgumentException("Unknown cloud: " + cloud); + throw new FlagValidationException("Unknown cloud: " + cloud); }); case CLUSTER_ID -> validateStringValues(condition, ClusterSpec.Id::from); case CLUSTER_TYPE -> validateStringValues(condition, ClusterSpec.Type::from); @@ -261,18 +240,18 @@ public class SystemFlagsDataArchive { case HOSTNAME -> validateStringValues(condition, HostName::of); case NODE_TYPE -> validateStringValues(condition, NodeType::valueOf); case SYSTEM -> validateStringValues(condition, system -> { - if (!Set.of(SystemName.cd, SystemName.main, SystemName.PublicCd, SystemName.Public).contains(SystemName.from(system))) - throw new IllegalArgumentException("Unknown system: " + system); + if (!SystemName.hostedVespa().contains(SystemName.from(system))) + throw new FlagValidationException("Unknown system: " + system); }); case TENANT_ID -> validateStringValues(condition, TenantName::from); case VESPA_VERSION -> validateStringValues(condition, versionString -> { - Version vespaVersion = Version.fromString(versionString); - if (vespaVersion.getMajor() < 8) - throw new IllegalArgumentException("Major Vespa version must be at least 8: " + versionString); + if (Version.fromString(versionString).getMajor() < 8) + throw new FlagValidationException("Major Vespa version must be at least 8: " + versionString); }); - case ZONE_ID -> validateStringValues(condition, zoneId -> { - if (!zones.contains(ZoneId.from(zoneId))) - throw new IllegalArgumentException("Unknown zone: " + zoneId); + case ZONE_ID -> validateStringValues(condition, zoneIdString -> { + ZoneId zoneId = ZoneId.from(zoneIdString); + if (!zones.isEmpty() && !zones.contains(zoneId)) + throw new FlagValidationException("Unknown zone: " + zoneIdString); }); } })); @@ -284,10 +263,16 @@ public class SystemFlagsDataArchive { .orElseThrow(() -> { String dimension = condition.get("dimension").asString().orElseThrow(); String type = condition.get("type").asString().orElseThrow(); - return new IllegalArgumentException("Non-string value in %s %s condition: %s".formatted( + return new FlagValidationException("Non-string %s in %s condition: %s".formatted( dimension, type, conditionValue)); }); - valueValidator.accept(value); + try { + valueValidator.accept(value); + } catch (IllegalArgumentException e) { + String dimension = condition.get("dimension").asString().orElseThrow(); + String type = condition.get("type").asString().orElseThrow(); + throw new FlagValidationException("Invalid %s '%s' in %s condition: %s".formatted(dimension, value, type, e.getMessage())); + } }); } diff --git a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/FlagsTargetTest.java b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/FlagsTargetTest.java new file mode 100644 index 00000000000..9177813e38f --- /dev/null +++ b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/FlagsTargetTest.java @@ -0,0 +1,41 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.api.systemflags.v1; + +import com.yahoo.config.provision.SystemName; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * @author hakonhall + */ +class FlagsTargetTest { + @Test + void sanityCheckFilename() { + assertTrue(FlagsTarget.filenameForSystem("default.json", SystemName.main)); + assertTrue(FlagsTarget.filenameForSystem("main.json", SystemName.main)); + assertTrue(FlagsTarget.filenameForSystem("main.controller.json", SystemName.main)); + assertTrue(FlagsTarget.filenameForSystem("main.prod.json", SystemName.main)); + assertTrue(FlagsTarget.filenameForSystem("main.prod.us-west-1.json", SystemName.main)); + assertTrue(FlagsTarget.filenameForSystem("main.prod.abc-foo-3.json", SystemName.main)); + + assertFalse(FlagsTarget.filenameForSystem("public.json", SystemName.main)); + assertFalse(FlagsTarget.filenameForSystem("public.controller.json", SystemName.main)); + assertFalse(FlagsTarget.filenameForSystem("public.prod.json", SystemName.main)); + assertFalse(FlagsTarget.filenameForSystem("public.prod.us-west-1.json", SystemName.main)); + assertFalse(FlagsTarget.filenameForSystem("public.prod.abc-foo-3.json", SystemName.main)); + + assertFlagValidationException("First part of flag filename is neither 'default' nor a valid system: defaults.json", "defaults.json"); + assertFlagValidationException("Invalid flag filename: default", "default"); + assertFlagValidationException("Invalid flag filename: README", "README"); + assertFlagValidationException("First part of flag filename is neither 'default' nor a valid system: nosystem.json", "nosystem.json"); + assertFlagValidationException("Invalid environment in flag filename: main.noenv.json", "main.noenv.json"); + assertFlagValidationException("Invalid region in flag filename: main.prod.%badregion.json", "main.prod.%badregion.json"); + } + + private void assertFlagValidationException(String expectedMessage, String filename) { + FlagValidationException e = assertThrows(FlagValidationException.class, () -> FlagsTarget.filenameForSystem(filename, SystemName.main)); + assertEquals(expectedMessage, e.getMessage()); + } + +}
\ No newline at end of file diff --git a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchiveTest.java b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchiveTest.java index 3417dc85224..2d0374dc888 100644 --- a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchiveTest.java +++ b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/systemflags/v1/SystemFlagsDataArchiveTest.java @@ -28,10 +28,8 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.net.URI; -import java.nio.file.Path; import java.nio.file.Paths; import java.util.List; -import java.util.Map; import java.util.Optional; import java.util.Set; @@ -76,40 +74,73 @@ public class SystemFlagsDataArchiveTest { @Test void can_serialize_and_deserialize_archive() throws IOException { + can_serialize_and_deserialize_archive(false); + can_serialize_and_deserialize_archive(true); + } + + private void can_serialize_and_deserialize_archive(boolean forceAddFiles) throws IOException { File tempFile = File.createTempFile("serialized-flags-archive", null, temporaryFolder); try (OutputStream out = new BufferedOutputStream(new FileOutputStream(tempFile))) { - var archive = SystemFlagsDataArchive.fromDirectory(Paths.get("src/test/resources/system-flags/")); + var archive = fromDirectory("system-flags", forceAddFiles); + if (forceAddFiles) + archive.validateAllFilesAreForTargets(Set.of(mainControllerTarget, prodUsWestCfgTarget)); archive.toZip(out); } try (InputStream in = new BufferedInputStream(new FileInputStream(tempFile))) { - SystemFlagsDataArchive archive = SystemFlagsDataArchive.fromZip(in); + SystemFlagsDataArchive archive = SystemFlagsDataArchive.fromZip(in, createZoneRegistryMock()); assertArchiveReturnsCorrectTestFlagDataForTarget(archive); } } @Test void retrieves_correct_flag_data_for_target() { - var archive = SystemFlagsDataArchive.fromDirectory(Paths.get("src/test/resources/system-flags/")); + retrieves_correct_flag_data_for_target(false); + retrieves_correct_flag_data_for_target(true); + } + + private void retrieves_correct_flag_data_for_target(boolean forceAddFiles) { + var archive = fromDirectory("system-flags", forceAddFiles); + if (forceAddFiles) + archive.validateAllFilesAreForTargets(Set.of(mainControllerTarget, prodUsWestCfgTarget)); assertArchiveReturnsCorrectTestFlagDataForTarget(archive); } @Test void supports_multi_level_flags_directory() { - var archive = SystemFlagsDataArchive.fromDirectory(Paths.get("src/test/resources/system-flags-multi-level/")); + supports_multi_level_flags_directory(false); + supports_multi_level_flags_directory(true); + } + + private void supports_multi_level_flags_directory(boolean forceAddFiles) { + var archive = fromDirectory("system-flags-multi-level", forceAddFiles); + if (forceAddFiles) + archive.validateAllFilesAreForTargets(Set.of(mainControllerTarget, prodUsWestCfgTarget)); assertFlagDataHasValue(archive, MY_TEST_FLAG, mainControllerTarget, "default"); } @Test void duplicated_flagdata_is_detected() { - Throwable exception = assertThrows(IllegalArgumentException.class, () -> { - var archive = SystemFlagsDataArchive.fromDirectory(Paths.get("src/test/resources/system-flags-multi-level-with-duplicated-flagdata/")); - }); + duplicated_flagdata_is_detected(false); + duplicated_flagdata_is_detected(true); + } + + private void duplicated_flagdata_is_detected(boolean forceAddFiles) { + Throwable exception = assertThrows(FlagValidationException.class, () -> { + fromDirectory("system-flags-multi-level-with-duplicated-flagdata", forceAddFiles); + }); assertTrue(exception.getMessage().contains("contains redundant flag data for id 'my-test-flag' already set in another directory!")); } @Test void empty_files_are_handled_as_no_flag_data_for_target() { - var archive = SystemFlagsDataArchive.fromDirectory(Paths.get("src/test/resources/system-flags/")); + empty_files_are_handled_as_no_flag_data_for_target(false); + empty_files_are_handled_as_no_flag_data_for_target(true); + } + + private void empty_files_are_handled_as_no_flag_data_for_target(boolean forceAddFiles) { + var archive = fromDirectory("system-flags", forceAddFiles); + if (forceAddFiles) + archive.validateAllFilesAreForTargets(Set.of(mainControllerTarget, prodUsWestCfgTarget)); assertNoFlagData(archive, FLAG_WITH_EMPTY_DATA, mainControllerTarget); assertFlagDataHasValue(archive, FLAG_WITH_EMPTY_DATA, prodUsWestCfgTarget, "main.prod.us-west-1"); assertNoFlagData(archive, FLAG_WITH_EMPTY_DATA, prodUsEast3CfgTarget); @@ -117,35 +148,43 @@ public class SystemFlagsDataArchiveTest { } @Test - void throws_exception_on_non_json_file() { - Throwable exception = assertThrows(IllegalArgumentException.class, () -> { - SystemFlagsDataArchive.fromDirectory(Paths.get("src/test/resources/system-flags-with-invalid-file-name/")); + void hv_throws_exception_on_non_json_file() { + Throwable exception = assertThrows(FlagValidationException.class, () -> { + fromDirectory("system-flags-with-invalid-file-name", false); }); - assertTrue(exception.getMessage().contains("Only JSON files are allowed in 'flags/' directory (found 'flags/my-test-flag/file-name-without-dot-json')")); + assertEquals("Invalid flag filename: file-name-without-dot-json", + exception.getMessage()); } @Test void throws_exception_on_unknown_file() { - Throwable exception = assertThrows(IllegalArgumentException.class, () -> { - SystemFlagsDataArchive archive = SystemFlagsDataArchive.fromDirectory(Paths.get("src/test/resources/system-flags-with-unknown-file-name/")); - archive.validateAllFilesAreForTargets(SystemName.main, Set.of(mainControllerTarget, prodUsWestCfgTarget)); + Throwable exception = assertThrows(FlagValidationException.class, () -> { + SystemFlagsDataArchive archive = fromDirectory("system-flags-with-unknown-file-name", true); + archive.validateAllFilesAreForTargets(Set.of(mainControllerTarget, prodUsWestCfgTarget)); }); - assertTrue(exception.getMessage().contains("Unknown flag file: flags/my-test-flag/main.prod.unknown-region.json")); + assertEquals("Unknown flag file: flags/my-test-flag/main.prod.unknown-region.json", exception.getMessage()); + } + + @Test + void unknown_region_is_still_zipped() { + // This is useful when the program zipping the files is on a different version than the controller + var archive = fromDirectory("system-flags-with-unknown-file-name", false); + assertTrue(archive.hasFlagData(MY_TEST_FLAG, "main.prod.unknown-region.json")); } @Test void throws_exception_on_unknown_region() { - Throwable exception = assertThrows(IllegalArgumentException.class, () -> { - Path directory = Paths.get("src/test/resources/system-flags-with-unknown-file-name/"); - SystemFlagsDataArchive.fromDirectoryAndSystem(directory, createZoneRegistryMock()); + Throwable exception = assertThrows(FlagValidationException.class, () -> { + var archive = fromDirectory("system-flags-with-unknown-file-name", true); + archive.validateAllFilesAreForTargets(Set.of(mainControllerTarget, prodUsWestCfgTarget)); }); - assertTrue(exception.getMessage().contains("Environment or zone in filename 'main.prod.unknown-region.json' does not exist")); + assertEquals("Unknown flag file: flags/my-test-flag/main.prod.unknown-region.json", exception.getMessage()); } @Test void throws_on_unknown_field() { - Throwable exception = assertThrows(IllegalArgumentException.class, () -> { - SystemFlagsDataArchive.fromDirectory(Paths.get("src/test/resources/system-flags-with-unknown-field-name/")); + Throwable exception = assertThrows(FlagValidationException.class, () -> { + fromDirectory("system-flags-with-unknown-field-name", true); }); assertEquals(""" flags/my-test-flag/main.prod.us-west-1.json contains unknown non-comment fields or rules with null values: after removing any comment fields the JSON is: @@ -160,7 +199,7 @@ public class SystemFlagsDataArchiveTest { @Test void handles_absent_rule_value() { - SystemFlagsDataArchive archive = SystemFlagsDataArchive.fromDirectory(Paths.get("src/test/resources/system-flags-with-null-value/")); + SystemFlagsDataArchive archive = fromDirectory("system-flags-with-null-value", true); // west has null value on first rule List<FlagData> westFlagData = archive.flagData(prodUsWestCfgTarget); @@ -305,17 +344,18 @@ public class SystemFlagsDataArchiveTest { @Test void normalize_json_fail_on_invalid_values() { - failNormalizeJson("application", "\"a.b.c\"", "Application ids must be on the form tenant:application:instance, but was a.b.c"); + failNormalizeJson("application", "\"a.b.c\"", "Invalid application 'a.b.c' in whitelist condition: Application ids must be on the form tenant:application:instance, but was a.b.c"); failNormalizeJson("cloud", "\"foo\"", "Unknown cloud: foo"); // failNormalizeJson("cluster-id", ... any String is valid - failNormalizeJson("cluster-type", "\"foo\"", "Illegal cluster type 'foo'"); - failNormalizeJson("console-user-email", "123", "Non-string value in console-user-email whitelist condition: 123"); - failNormalizeJson("environment", "\"foo\"", "'foo' is not a valid environment identifier"); - failNormalizeJson("hostname", "\"not:a:hostname\"", "hostname must match '(([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9-]{0,61}[A-Za-z0-9])\\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9-]{0,61}[A-Za-z0-9])\\.?', but got: 'not:a:hostname'"); - failNormalizeJson("node-type", "\"footype\"", "No enum constant com.yahoo.config.provision.NodeType.footype"); - failNormalizeJson("system", "\"bar\"", "'bar' is not a valid system"); - failNormalizeJson("tenant", "123", "Non-string value in tenant whitelist condition: 123"); - failNormalizeJson("vespa-version", "\"not-a-version\"", "Invalid version component in 'not-a-version'"); + failNormalizeJson("cluster-type", "\"foo\"", "Invalid cluster-type 'foo' in whitelist condition: Illegal cluster type 'foo'"); + failNormalizeJson("console-user-email", "123", "Non-string console-user-email in whitelist condition: 123"); + failNormalizeJson("environment", "\"foo\"", "Invalid environment 'foo' in whitelist condition: 'foo' is not a valid environment identifier"); + failNormalizeJson("hostname", "\"not:a:hostname\"", "Invalid hostname 'not:a:hostname' in whitelist condition: hostname must match '(([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9-]{0,61}[A-Za-z0-9])\\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9-]{0,61}[A-Za-z0-9])\\.?', but got: 'not:a:hostname'"); + failNormalizeJson("node-type", "\"footype\"", "Invalid node-type 'footype' in whitelist condition: No enum constant com.yahoo.config.provision.NodeType.footype"); + failNormalizeJson("system", "\"bar\"", "Invalid system 'bar' in whitelist condition: 'bar' is not a valid system"); + failNormalizeJson("tenant", "123", "Non-string tenant in whitelist condition: 123"); + failNormalizeJson("vespa-version", "\"not-a-version\"", "Invalid vespa-version 'not-a-version' in whitelist condition: Invalid version component in 'not-a-version'"); + failNormalizeJson("zone", "\"dev.%illegal\"", Set.of(ZoneId.from("prod.example-region")), "Invalid zone 'dev.%illegal' in whitelist condition: region name must match '[a-z]([a-z0-9-]*[a-z0-9])*', but got: '%illegal'"); failNormalizeJson("zone", "\"dev.non-existing-zone\"", Set.of(ZoneId.from("prod.example-region")), "Unknown zone: dev.non-existing-zone"); } @@ -334,14 +374,16 @@ public class SystemFlagsDataArchiveTest { @Test void ignores_files_not_related_to_specified_system_definition() { - ZoneRegistry registry = createZoneRegistryMock(); - Path testDirectory = Paths.get("src/test/resources/system-flags-for-multiple-systems/"); - var archive = SystemFlagsDataArchive.fromDirectoryAndSystem(testDirectory, registry); + var archive = fromDirectory("system-flags-for-multiple-systems", false); assertFlagDataHasValue(archive, MY_TEST_FLAG, cdControllerTarget, "default"); // Would be 'cd.controller' if files for CD system were included assertFlagDataHasValue(archive, MY_TEST_FLAG, mainControllerTarget, "default"); assertFlagDataHasValue(archive, MY_TEST_FLAG, prodUsWestCfgTarget, "main.prod.us-west-1"); } + private SystemFlagsDataArchive fromDirectory(String testDirectory, boolean forceAddFiles) { + return SystemFlagsDataArchive.fromDirectory(Paths.get("src/test/resources/" + testDirectory), createZoneRegistryMock(), forceAddFiles); + } + @SuppressWarnings("unchecked") // workaround for mocking a method for generic return type private static ZoneRegistry createZoneRegistryMock() { // Cannot use the standard registry mock as it's located in controller-server module @@ -373,7 +415,7 @@ public class SystemFlagsDataArchiveTest { List<FlagData> data = getData(archive, flagId, target); assertEquals(1, data.size()); FlagData flagData = data.get(0); - RawFlag rawFlag = flagData.resolve(FetchVector.fromMap(Map.of())).get(); + RawFlag rawFlag = flagData.resolve(new FetchVector()).get(); assertEquals(String.format("\"%s\"", value), rawFlag.asJson()); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployer.java index 355f06fc753..2c38066eddd 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsDeployer.java @@ -8,6 +8,7 @@ import com.yahoo.vespa.flags.FlagId; import com.yahoo.vespa.flags.Flags; import com.yahoo.vespa.flags.json.FlagData; import com.yahoo.vespa.hosted.controller.api.integration.ControllerIdentityProvider; +import com.yahoo.vespa.hosted.controller.api.systemflags.v1.FlagValidationException; import com.yahoo.vespa.hosted.controller.api.systemflags.v1.FlagsTarget; import com.yahoo.vespa.hosted.controller.api.systemflags.v1.SystemFlagsDataArchive; import com.yahoo.vespa.hosted.controller.restapi.systemflags.SystemFlagsDeployResult.OperationError; @@ -57,6 +58,12 @@ class SystemFlagsDeployer { } SystemFlagsDeployResult deployFlags(SystemFlagsDataArchive archive, boolean dryRun) { + try { + archive.validateAllFilesAreForTargets(targets); + } catch (FlagValidationException e) { + return new SystemFlagsDeployResult(List.of(OperationError.archiveValidationFailed(e.getMessage()))); + } + Map<FlagsTarget, Future<SystemFlagsDeployResult>> futures = new HashMap<>(); for (FlagsTarget target : targets) { futures.put(target, executor.submit(() -> deployFlags(target, archive.flagData(target), dryRun))); @@ -70,11 +77,6 @@ class SystemFlagsDeployer { throw new RuntimeException(e); } }); - try { - archive.validateAllFilesAreForTargets(system, targets); - } catch (IllegalArgumentException e) { - results.add(new SystemFlagsDeployResult(List.of(OperationError.archiveValidationFailed(e.getMessage())))); - } return SystemFlagsDeployResult.merge(results); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsHandler.java index e9b087690ff..bb285b8b742 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/systemflags/SystemFlagsHandler.java @@ -10,10 +10,13 @@ import com.yahoo.restapi.JacksonJsonResponse; import com.yahoo.restapi.Path; import com.yahoo.vespa.hosted.controller.api.integration.ControllerIdentityProvider; import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneRegistry; +import com.yahoo.vespa.hosted.controller.api.systemflags.v1.FlagValidationException; import com.yahoo.vespa.hosted.controller.api.systemflags.v1.FlagsTarget; import com.yahoo.vespa.hosted.controller.api.systemflags.v1.SystemFlagsDataArchive; import com.yahoo.vespa.hosted.controller.restapi.ErrorResponses; +import java.io.InputStream; +import java.util.List; import java.util.concurrent.Executor; /** @@ -27,12 +30,14 @@ public class SystemFlagsHandler extends ThreadedHttpRequestHandler { private static final String API_PREFIX = "/system-flags/v1"; private final SystemFlagsDeployer deployer; + private final ZoneRegistry zoneRegistry; @Inject public SystemFlagsHandler(ZoneRegistry zoneRegistry, ControllerIdentityProvider identityProvider, Executor executor) { super(executor); + this.zoneRegistry = zoneRegistry; this.deployer = new SystemFlagsDeployer(identityProvider, zoneRegistry.system(), FlagsTarget.getAllTargetsInSystem(zoneRegistry, true)); } @@ -57,12 +62,22 @@ public class SystemFlagsHandler extends ThreadedHttpRequestHandler { if (!contentType.equalsIgnoreCase("application/zip")) { return ErrorResponse.badRequest("Invalid content type: " + contentType); } - SystemFlagsDataArchive archive = SystemFlagsDataArchive.fromZip(request.getData()); - SystemFlagsDeployResult result = deployer.deployFlags(archive, dryRun); + SystemFlagsDeployResult result = deploy(request.getData(), dryRun); return new JacksonJsonResponse<>(200, result.toWire()); } catch (Exception e) { return ErrorResponses.logThrowing(request, log, e); } } + private SystemFlagsDeployResult deploy(InputStream zipStream, boolean dryRun) { + SystemFlagsDataArchive archive; + try { + archive = SystemFlagsDataArchive.fromZip(zipStream, zoneRegistry); + } catch (FlagValidationException e) { + return new SystemFlagsDeployResult(List.of(SystemFlagsDeployResult.OperationError.archiveValidationFailed(e.getMessage()))); + } + + return deployer.deployFlags(archive, dryRun); + } + } diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java index 26e56a8e482..c6d141764fb 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -14,8 +14,6 @@ import java.util.TreeMap; import java.util.function.Predicate; import static com.yahoo.vespa.flags.FetchVector.Dimension.APPLICATION_ID; -import static com.yahoo.vespa.flags.FetchVector.Dimension.CLUSTER_ID; -import static com.yahoo.vespa.flags.FetchVector.Dimension.CLUSTER_TYPE; import static com.yahoo.vespa.flags.FetchVector.Dimension.CONSOLE_USER_EMAIL; import static com.yahoo.vespa.flags.FetchVector.Dimension.HOSTNAME; import static com.yahoo.vespa.flags.FetchVector.Dimension.NODE_TYPE; @@ -47,15 +45,6 @@ public class Flags { private static volatile TreeMap<FlagId, FlagDefinition> flags = new TreeMap<>(); - public static final UnboundBooleanFlag DROP_CACHES = defineFeatureFlag( - "drop-caches", false, - List.of("hakonhall", "baldersheim"), "2023-03-06", "2023-08-05", - "Drop caches on tenant hosts", - "Takes effect on next tick", - // The application ID is the exclusive application ID associated with the host, - // if any, or otherwise hosted-vespa:tenant-host:default. - APPLICATION_ID, TENANT_ID, CLUSTER_ID, CLUSTER_TYPE); - public static final UnboundDoubleFlag DEFAULT_TERM_WISE_LIMIT = defineDoubleFlag( "default-term-wise-limit", 1.0, List.of("baldersheim"), "2020-12-02", "2023-12-31", @@ -313,14 +302,6 @@ public class Flags { "Takes effect at redeployment", APPLICATION_ID); - public static final UnboundBooleanFlag NODE_ADMIN_TENANT_SERVICE_REGISTRY = defineFeatureFlag( - "node-admin-tenant-service-registry", true, - List.of("olaa"), "2023-04-12", "2023-08-07", - "Whether AthenzCredentialsMaintainer in node-admin should create tenant service identity certificate", - "Takes effect on next tick", - HOSTNAME, VESPA_VERSION, APPLICATION_ID - ); - public static final UnboundBooleanFlag ENABLE_CROWDSTRIKE = defineFeatureFlag( "enable-crowdstrike", true, List.of("andreer"), "2023-04-13", "2023-08-31", "Whether to enable CrowdStrike.", "Takes effect on next host admin tick", diff --git a/flags/src/main/java/com/yahoo/vespa/flags/PermanentFlags.java b/flags/src/main/java/com/yahoo/vespa/flags/PermanentFlags.java index 8823fc47d92..619dd39ca47 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/PermanentFlags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/PermanentFlags.java @@ -376,6 +376,14 @@ public class PermanentFlags { "triggered", "Takes effect immediately"); + public static final UnboundBooleanFlag DROP_CACHES = defineFeatureFlag( + "drop-caches", false, + "Drop caches on tenant hosts", + "Takes effect on next tick", + // The application ID is the exclusive application ID associated with the host, + // if any, or otherwise hosted-vespa:tenant-host:default. + APPLICATION_ID, TENANT_ID, CLUSTER_ID, CLUSTER_TYPE); + private PermanentFlags() {} private static UnboundBooleanFlag defineFeatureFlag( diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/cgroup/IoController.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/cgroup/IoController.java index d3b8c445520..5bbdd5c3b70 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/cgroup/IoController.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/cgroup/IoController.java @@ -37,9 +37,6 @@ public class IoController { String[] parts = device.split(":"); return new Device(parseInt(parts[0]), parseInt(parts[1])); } - public static Device fromDeviceNumber(int deviceNumber) { - return new Device(deviceNumber >>> 8, deviceNumber & 0xFF); - } @Override public int compareTo(Device o) { diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/identity/AthenzCredentialsMaintainer.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/identity/AthenzCredentialsMaintainer.java index b6ec0ebbd94..830b7f4ed33 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/identity/AthenzCredentialsMaintainer.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/identity/AthenzCredentialsMaintainer.java @@ -80,7 +80,6 @@ public class AthenzCredentialsMaintainer implements CredentialsMaintainer { private final String certificateDnsSuffix; private final ServiceIdentityProvider hostIdentityProvider; private final IdentityDocumentClient identityDocumentClient; - private final BooleanFlag tenantServiceIdentityFlag; // Used as an optimization to ensure ZTS is not DDoS'ed on continuously failing refresh attempts private final Map<ContainerName, Instant> lastRefreshAttempt = new ConcurrentHashMap<>(); @@ -89,7 +88,6 @@ public class AthenzCredentialsMaintainer implements CredentialsMaintainer { ConfigServerInfo configServerInfo, String certificateDnsSuffix, ServiceIdentityProvider hostIdentityProvider, - FlagSource flagSource, Timer timer) { this.ztsTrustStorePath = ztsTrustStorePath; this.certificateDnsSuffix = certificateDnsSuffix; @@ -99,7 +97,6 @@ public class AthenzCredentialsMaintainer implements CredentialsMaintainer { hostIdentityProvider, new AthenzIdentityVerifier(Set.of(configServerInfo.getConfigServerIdentity()))); this.timer = timer; - this.tenantServiceIdentityFlag = Flags.NODE_ADMIN_TENANT_SERVICE_REGISTRY.bindTo(flagSource); } public boolean converge(NodeAgentContext context) { @@ -109,11 +106,7 @@ public class AthenzCredentialsMaintainer implements CredentialsMaintainer { if (context.zone().getSystemName().isPublic()) return modified; - if (shouldWriteTenantServiceIdentity(context)) { - modified |= maintain(context, TENANT); - } else { - modified |= deleteTenantCredentials(context); - } + modified |= maintain(context, TENANT); return modified; } @@ -268,24 +261,6 @@ public class AthenzCredentialsMaintainer implements CredentialsMaintainer { return "node-certificate"; } - private boolean deleteTenantCredentials(NodeAgentContext context) { - var siaDirectory = context.paths().of(CONTAINER_SIA_DIRECTORY, context.users().vespa()); - var identityDocumentFile = siaDirectory.resolve(TENANT.getIdentityDocument()); - if (!Files.exists(identityDocumentFile)) return false; - return getAthenzIdentity(context, TENANT, identityDocumentFile).map(athenzIdentity -> { - var privateKeyFile = (ContainerPath) SiaUtils.getPrivateKeyFile(siaDirectory, athenzIdentity); - var certificateFile = (ContainerPath) SiaUtils.getCertificateFile(siaDirectory, athenzIdentity); - try { - var modified = Files.deleteIfExists(identityDocumentFile); - modified |= Files.deleteIfExists(privateKeyFile); - modified |= Files.deleteIfExists(certificateFile); - return modified; - } catch (IOException e) { - throw new UncheckedIOException(e); - } - }).orElse(false); - } - private boolean shouldRefreshCredentials(Duration age) { return age.compareTo(REFRESH_PERIOD) >= 0; } @@ -399,16 +374,6 @@ public class AthenzCredentialsMaintainer implements CredentialsMaintainer { } } - private boolean shouldWriteTenantServiceIdentity(NodeAgentContext context) { - var version = context.node().currentVespaVersion() - .orElse(context.node().wantedVespaVersion().orElse(Version.emptyVersion)); - var appId = context.node().owner().orElse(ApplicationId.defaultId()); - return tenantServiceIdentityFlag - .with(FetchVector.Dimension.VESPA_VERSION, version.toFullString()) - .with(FetchVector.Dimension.APPLICATION_ID, appId.serializedForm()) - .value(); - } - private void copyCredsToLegacyPath(NodeAgentContext context, ContainerPath privateKeyFile, ContainerPath certificateFile) throws IOException { var legacySiaDirectory = context.paths().of(LEGACY_SIA_DIRECTORY, context.users().vespa()); var keysDirectory = legacySiaDirectory.resolve("keys"); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileAttributes.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileAttributes.java index 332b4e61dc1..c638fe98cdf 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileAttributes.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileAttributes.java @@ -13,36 +13,13 @@ import java.util.Set; * * @author hakonhall */ -public class FileAttributes { - - private final Instant lastModifiedTime; - private final int ownerId; - private final int groupId; - private final String permissions; - private final boolean isRegularFile; - private final boolean isDirectory; - private final long size; - - public FileAttributes(Instant lastModifiedTime, int ownerId, int groupId, String permissions, boolean isRegularFile, boolean isDirectory, long size) { - this.lastModifiedTime = lastModifiedTime; - this.ownerId = ownerId; - this.groupId = groupId; - this.permissions = permissions; - this.isRegularFile = isRegularFile; - this.isDirectory = isDirectory; - this.size = size; - } - - public Instant lastModifiedTime() { return lastModifiedTime; } - public int ownerId() { return ownerId; } - public int groupId() { return groupId; } - public String permissions() { return permissions; } - public boolean isRegularFile() { return isRegularFile; } - public boolean isDirectory() { return isDirectory; } - public long size() { return size; } +public record FileAttributes(Instant lastModifiedTime, int ownerId, int groupId, String permissions, + boolean isRegularFile, boolean isDirectory, long size, int deviceMajor, int deviceMinor) { @SuppressWarnings("unchecked") static FileAttributes fromAttributes(Map<String, Object> attributes) { + long dev_t = (long) attributes.get("dev"); + return new FileAttributes( ((FileTime) attributes.get("lastModifiedTime")).toInstant(), (int) attributes.get("uid"), @@ -50,6 +27,11 @@ public class FileAttributes { PosixFilePermissions.toString(((Set<PosixFilePermission>) attributes.get("permissions"))), (boolean) attributes.get("isRegularFile"), (boolean) attributes.get("isDirectory"), - (long) attributes.get("size")); + (long) attributes.get("size"), + deviceMajor(dev_t), deviceMinor(dev_t)); } + + // Encoded as MMMM Mmmm mmmM MMmm, where M is a hex digit of the major number and m is a hex digit of the minor number. + static int deviceMajor(long dev_t) { return (int) (((dev_t & 0xFFFFF00000000000L) >> 32) | ((dev_t & 0xFFF00) >> 8)); } + static int deviceMinor(long dev_t) { return (int) (((dev_t & 0x00000FFFFFF00000L) >> 12) | (dev_t & 0x000FF)); } } diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/cgroup/IoControllerTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/cgroup/IoControllerTest.java index d2a4ebbfbbd..71a05eb4571 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/cgroup/IoControllerTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/cgroup/IoControllerTest.java @@ -3,7 +3,6 @@ package com.yahoo.vespa.hosted.node.admin.cgroup; import org.junit.jupiter.api.Test; -import static com.yahoo.vespa.hosted.node.admin.cgroup.IoController.Device; import static com.yahoo.vespa.hosted.node.admin.cgroup.IoController.Max; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -13,12 +12,6 @@ import static org.junit.jupiter.api.Assertions.assertEquals; class IoControllerTest { @Test - void device_number_parsing() { - assertEquals(new Device(253, 15), Device.fromDeviceNumber(253 * 256 + 15)); - assertEquals(new Device(345, 123), Device.fromDeviceNumber(345 * 256 + 123)); - } - - @Test void parse_io_max() { assertEquals(Max.UNLIMITED, Max.fromString("")); assertEquals(new Max(Size.from(1), Size.max(), Size.max(), Size.max()), Max.fromString("rbps=1 wiops=max")); diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileAttributesCacheTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileAttributesCacheTest.java index 8c9188a9409..1b68d1d10a3 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileAttributesCacheTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileAttributesCacheTest.java @@ -3,9 +3,12 @@ package com.yahoo.vespa.hosted.node.admin.task.util.file; import org.junit.jupiter.api.Test; +import java.time.Instant; import java.util.Optional; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -23,7 +26,8 @@ public class FileAttributesCacheTest { verify(unixPath, times(1)).getAttributesIfExists(); verifyNoMoreInteractions(unixPath); - FileAttributes attributes = mock(FileAttributes.class); + FileAttributes attributes = new FileAttributes(Instant.EPOCH, 0, 0, "", false, false, 0, 0, 0); + when(unixPath.getAttributesIfExists()).thenReturn(Optional.of(attributes)); when(unixPath.getAttributesIfExists()).thenReturn(Optional.of(attributes)); assertTrue(cache.get().isPresent()); verify(unixPath, times(1 + 1)).getAttributesIfExists(); @@ -32,4 +36,4 @@ public class FileAttributesCacheTest { assertEquals(attributes, cache.getOrThrow()); verifyNoMoreInteractions(unixPath); } -}
\ No newline at end of file +} diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileAttributesTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileAttributesTest.java new file mode 100644 index 00000000000..ddcd225a871 --- /dev/null +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileAttributesTest.java @@ -0,0 +1,20 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.node.admin.task.util.file; + +import org.junit.jupiter.api.Test; + +import static com.yahoo.vespa.hosted.node.admin.task.util.file.FileAttributes.deviceMajor; +import static com.yahoo.vespa.hosted.node.admin.task.util.file.FileAttributes.deviceMinor; +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** + * @author freva + */ +class FileAttributesTest { + + @Test + void parse_dev_t() { + assertEquals(0x12345BCD, deviceMajor(0x1234567890ABCDEFL)); + assertEquals(0x67890AEF, deviceMinor(0x1234567890ABCDEFL)); + } +} |