diff options
33 files changed, 257 insertions, 195 deletions
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/RunDataStore.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/RunDataStore.java index fe936b3a752..418d1089428 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/RunDataStore.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/RunDataStore.java @@ -17,6 +17,9 @@ public interface RunDataStore { /** Stores the given log for the given deployment job. */ void put(RunId id, byte[] log); + /** Deletes the run logs for the given deployment job. */ + void delete(RunId id); + /** Deletes all data associated with the given application. */ void delete(ApplicationId id); diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockRunDataStore.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockRunDataStore.java index 0d7d8eca10e..16874f996a5 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockRunDataStore.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockRunDataStore.java @@ -27,6 +27,11 @@ public class MockRunDataStore implements RunDataStore { } @Override + public void delete(RunId id) { + logs.remove(id); + } + + @Override public void delete(ApplicationId id) { logs.keySet().removeIf(runId -> runId.application().equals(id)); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/mock/ZtsClientMock.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/mock/ZtsClientMock.java index 8b3fb3ca47e..4c586d660e7 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/mock/ZtsClientMock.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/mock/ZtsClientMock.java @@ -73,12 +73,12 @@ public class ZtsClientMock implements ZtsClient { } @Override - public X509Certificate getRoleCertificate(AthenzRole role, Duration expiry, KeyPair keyPair, String cloud) { + public X509Certificate getRoleCertificate(AthenzRole role, Pkcs10Csr csr, Duration expiry) { throw new UnsupportedOperationException(); } @Override - public X509Certificate getRoleCertificate(AthenzRole role, KeyPair keyPair, String cloud) { + public X509Certificate getRoleCertificate(AthenzRole role, Pkcs10Csr csr) { throw new UnsupportedOperationException(); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java index 72279be921e..453447830bb 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java @@ -25,10 +25,12 @@ import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; import java.net.URI; import java.util.Collections; import java.util.HashSet; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.SortedMap; import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; @@ -55,6 +57,8 @@ import static com.yahoo.vespa.hosted.controller.deployment.Step.endTests; */ public class JobController { + private static final int historyLength = 256; + private final Controller controller; private final CuratorDb curator; private final BufferedLogStore logs; @@ -67,15 +71,14 @@ public class JobController { this.cloud = testerCloud; } - public TesterCloud cloud() { - return cloud; - } + public TesterCloud cloud() { return cloud; } + public int historyLength() { return historyLength; } /** Rewrite all job data with the newest format. */ public void updateStorage() { for (ApplicationId id : applications()) for (JobType type : jobs(id)) { - locked(id, type, runs -> { // runs is unmodified, and written back as such. + locked(id, type, runs -> { // runs is not modified here, and is written as it was. curator.readLastRun(id, type).ifPresent(curator::writeLastRun); }); } @@ -110,23 +113,21 @@ public class JobController { /** Fetches any new test log entries, and records the id of the last of these, for continuation. */ public void updateTestLog(RunId id) { - try (Lock __ = curator.lock(id.application(), id.type())) { - active(id).ifPresent(run -> { + locked(id, run -> { if ( ! run.readySteps().contains(endTests)) - return; + return run; Optional<URI> testerEndpoint = testerEndpoint(id); if ( ! testerEndpoint.isPresent()) - return; + return run; List<LogEntry> entries = cloud.getLog(testerEndpoint.get(), run.lastTestLogEntry()); if (entries.isEmpty()) - return; + return run; logs.append(id.application(), id.type(), endTests, entries); - curator.writeLastRun(run.with(entries.stream().mapToLong(LogEntry::id).max().getAsLong())); + return run.with(entries.stream().mapToLong(LogEntry::id).max().getAsLong()); }); - } } /** Returns a list of all application which have registered. */ @@ -187,9 +188,17 @@ public class JobController { /** Changes the status of the given run to inactive, and stores it as a historic run. */ public void finish(RunId id) { - locked(id, run -> { // Store the modified run after it has been written to the collection, in case the latter fails. + locked(id, run -> { // Store the modified run after it has been written to history, in case the latter fails. Run finishedRun = run.finished(controller.clock().instant()); - locked(id.application(), id.type(), runs -> runs.put(run.id(), finishedRun)); + locked(id.application(), id.type(), runs -> { + runs.put(run.id(), finishedRun); + long last = id.number(); + Iterator<RunId> ids = runs.keySet().iterator(); + for (RunId old = ids.next(); old.number() <= last - historyLength; old = ids.next()) { + logs.delete(old); + ids.remove(); + } + }); logs.flush(id); return finishedRun; }); @@ -256,11 +265,7 @@ public class JobController { public void unregister(ApplicationId id) { controller.applications().lockIfPresent(id, application -> { controller.applications().store(application.withBuiltInternally(false)); - jobs(id).forEach(type -> { - try (Lock __ = curator.lock(id, type)) { - last(id, type).ifPresent(last -> active(last.id()).ifPresent(active -> abort(active.id()))); - } - }); + jobs(id).forEach(type -> last(id, type).ifPresent(last -> abort(last.id()))); }); } @@ -336,9 +341,9 @@ public class JobController { } /** Locks and modifies the list of historic runs for the given application and job type. */ - private void locked(ApplicationId id, JobType type, Consumer<Map<RunId, Run>> modifications) { + private void locked(ApplicationId id, JobType type, Consumer<SortedMap<RunId, Run>> modifications) { try (Lock __ = curator.lock(id, type)) { - Map<RunId, Run> runs = curator.readHistoricRuns(id, type); + SortedMap<RunId, Run> runs = curator.readHistoricRuns(id, type); modifications.accept(runs); curator.writeHistoricRuns(id, type, runs.values()); } @@ -347,9 +352,10 @@ public class JobController { /** Locks and modifies the run with the given id, provided it is still active. */ private void locked(RunId id, UnaryOperator<Run> modifications) { try (Lock __ = curator.lock(id.application(), id.type())) { - Run run = active(id).orElseThrow(() -> new IllegalArgumentException(id + " is not an active run!")); - run = modifications.apply(run); - curator.writeLastRun(run); + active(id).ifPresent(run -> { + run = modifications.apply(run); + curator.writeLastRun(run); + }); } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunner.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunner.java index d631df2921e..089f3012e7e 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunner.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunner.java @@ -75,7 +75,7 @@ public class JobRunner extends Maintainer { advance(jobs.run(run.id()).get()); } else if (run.readySteps().isEmpty()) - jobs.finish(run.id()); + executors.execute(() -> jobs.finish(run.id())); else run.readySteps().forEach(step -> executors.execute(() -> advance(run.id(), step))); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/BufferedLogStore.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/BufferedLogStore.java index a49061c9d67..3df4451f900 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/BufferedLogStore.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/BufferedLogStore.java @@ -77,6 +77,11 @@ public class BufferedLogStore { buffer.deleteLog(id.application(), id.type()); } + /** Deletes the logs for the given run, if already moved to storage. */ + public void delete(RunId id) { + store.delete(id); + } + /** Deletes all logs for the given application. */ public void delete(ApplicationId id) { for (JobType type : JobType.values()) diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java index 0d8ea8d2537..9ee477e5ab2 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java @@ -33,11 +33,12 @@ import java.time.Duration; import java.util.Arrays; import java.util.Collections; import java.util.HashSet; -import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.SortedMap; +import java.util.TreeMap; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.TimeoutException; import java.util.function.Function; @@ -47,6 +48,7 @@ import java.util.logging.Logger; import java.util.stream.Collectors; import java.util.stream.LongStream; +import static java.util.Comparator.comparing; import static java.util.stream.Collectors.collectingAndThen; /** @@ -365,9 +367,8 @@ public class CuratorDb { return readSlime(lastRunPath(id, type)).map(runSerializer::runFromSlime); } - public Map<RunId, Run> readHistoricRuns(ApplicationId id, JobType type) { - // TODO jvenstad: Add, somewhere, a retention filter based on age or count. - return readSlime(runsPath(id, type)).map(runSerializer::runsFromSlime).orElse(new LinkedHashMap<>()); + public SortedMap<RunId, Run> readHistoricRuns(ApplicationId id, JobType type) { + return readSlime(runsPath(id, type)).map(runSerializer::runsFromSlime).orElse(new TreeMap<>(comparing(RunId::number))); } public void deleteRunData(ApplicationId id, JobType type) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializer.java index b94a3b16898..5103dc244ed 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializer.java @@ -20,9 +20,9 @@ import com.yahoo.vespa.hosted.controller.deployment.Versions; import java.time.Instant; import java.util.EnumMap; -import java.util.LinkedHashMap; -import java.util.Map; import java.util.Optional; +import java.util.SortedMap; +import java.util.TreeMap; import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.aborted; import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.deploymentFailed; @@ -46,6 +46,7 @@ import static com.yahoo.vespa.hosted.controller.deployment.Step.installTester; import static com.yahoo.vespa.hosted.controller.deployment.Step.report; import static com.yahoo.vespa.hosted.controller.deployment.Step.startTests; import static com.yahoo.vespa.hosted.controller.deployment.Step.endTests; +import static java.util.Comparator.comparing; /** * Serialises and deserialises RunStatus objects for persistent storage. @@ -74,8 +75,8 @@ class RunSerializer { return runFromSlime(slime.get()); } - Map<RunId, Run> runsFromSlime(Slime slime) { - Map<RunId, Run> runs = new LinkedHashMap<>(); + SortedMap<RunId, Run> runsFromSlime(Slime slime) { + SortedMap<RunId, Run> runs = new TreeMap<>(comparing(RunId::number)); Inspector runArray = slime.get(); runArray.traverse((ArrayTraverser) (__, runObject) -> { Run run = runFromSlime(runObject); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java index 3f336087d05..3e87dcad053 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/JobRunnerTest.java @@ -229,6 +229,33 @@ public class JobRunnerTest { } @Test + public void historyPruning() { + DeploymentTester tester = new DeploymentTester(); + JobController jobs = tester.controller().jobController(); + JobRunner runner = new JobRunner(tester.controller(), Duration.ofDays(1), new JobControl(tester.controller().curator()), + inThreadExecutor(), (id, step) -> Optional.of(running)); + + ApplicationId id = tester.createApplication("real", "tenant", 1, 1L).id(); + jobs.submit(id, versions.targetApplication().source().get(), new byte[0], new byte[0]); + + for (int i = 0; i < jobs.historyLength(); i++) { + jobs.start(id, systemTest, versions); + runner.run(); + } + + assertEquals(256, jobs.runs(id, systemTest).size()); + assertTrue(jobs.details(new RunId(id, systemTest, 1)).isPresent()); + + jobs.start(id, systemTest, versions); + runner.run(); + + assertEquals(256, jobs.runs(id, systemTest).size()); + assertEquals(2, jobs.runs(id, systemTest).keySet().iterator().next().number()); + assertFalse(jobs.details(new RunId(id, systemTest, 1)).isPresent()); + assertTrue(jobs.details(new RunId(id, systemTest, 257)).isPresent()); + } + + @Test public void timeout() { DeploymentTester tester = new DeploymentTester(); JobController jobs = tester.controller().jobController(); diff --git a/linguistics/src/main/java/com/yahoo/language/process/GramSplitter.java b/linguistics/src/main/java/com/yahoo/language/process/GramSplitter.java index 3a4558b3cb9..94fd0e08493 100644 --- a/linguistics/src/main/java/com/yahoo/language/process/GramSplitter.java +++ b/linguistics/src/main/java/com/yahoo/language/process/GramSplitter.java @@ -1,7 +1,12 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.language.process; -import java.util.*; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.Iterator; +import java.util.List; +import java.util.NoSuchElementException; /** * A class which splits consecutive word character sequences into overlapping character n-grams. diff --git a/linguistics/src/main/java/com/yahoo/language/simple/SimpleLinguistics.java b/linguistics/src/main/java/com/yahoo/language/simple/SimpleLinguistics.java index d7246db6041..8cbbdeeae1d 100644 --- a/linguistics/src/main/java/com/yahoo/language/simple/SimpleLinguistics.java +++ b/linguistics/src/main/java/com/yahoo/language/simple/SimpleLinguistics.java @@ -33,27 +33,21 @@ public class SimpleLinguistics implements Linguistics { @Inject public SimpleLinguistics() { - CharacterClasses characterClasses = new CharacterClasses(); - this.normalizer = new SimpleNormalizer(); - this.transformer = new SimpleTransformer(); - this.detector = new SimpleDetector(); - this.characterClasses = new CharacterClasses(); - this.gramSplitter = new GramSplitter(characterClasses); + this(true); + } public SimpleLinguistics(boolean enableOptimaize) { - CharacterClasses characterClasses = new CharacterClasses(); - this.normalizer = new SimpleNormalizer(); - this.transformer = new SimpleTransformer(); - this.detector = new SimpleDetector(enableOptimaize); - this.characterClasses = new CharacterClasses(); - this.gramSplitter = new GramSplitter(characterClasses); + this(new SimpleDetector(enableOptimaize)); } public SimpleLinguistics(SimpleLinguisticsConfig config) { - CharacterClasses characterClasses = new CharacterClasses(); + this(new SimpleDetector(config.detector())); + } + + private SimpleLinguistics(Detector detector) { this.normalizer = new SimpleNormalizer(); this.transformer = new SimpleTransformer(); - this.detector = new SimpleDetector(config.detector()); + this.detector = detector; this.characterClasses = new CharacterClasses(); this.gramSplitter = new GramSplitter(characterClasses); } diff --git a/metrics/src/tests/metricmanagertest.cpp b/metrics/src/tests/metricmanagertest.cpp index e9934b6dbbb..f5d3a77d671 100644 --- a/metrics/src/tests/metricmanagertest.cpp +++ b/metrics/src/tests/metricmanagertest.cpp @@ -68,7 +68,7 @@ struct SubMetricSet : public MetricSet SubMetricSet::SubMetricSet(const Metric::String & name, MetricSet* owner) - : MetricSet(name, "sub", "sub desc", owner, "sub"), + : MetricSet(name, "sub", "sub desc", owner), val1("val1", "tag4 snaptest", "val1 desc", this), val2("val2", "tag5", "val2 desc", this), valsum("valsum", "tag4 snaptest", "valsum desc", this) diff --git a/metrics/src/vespa/metrics/jsonwriter.cpp b/metrics/src/vespa/metrics/jsonwriter.cpp index 8dd5e8fdbcb..6ea585f0635 100644 --- a/metrics/src/vespa/metrics/jsonwriter.cpp +++ b/metrics/src/vespa/metrics/jsonwriter.cpp @@ -41,26 +41,10 @@ JsonWriter::doneVisitingSnapshot(const MetricSnapshot&) _period = 0; } -void -JsonWriter::pushLegacyDimensionFromSet(const MetricSet& ms) -{ - _dimensionStack.push_back({{ms.getDimensionKey(), ms.getName()}}); -} - -void -JsonWriter::pushAllDimensionsFromSet(const MetricSet& ms) -{ - _dimensionStack.push_back(ms.getTags()); -} - bool JsonWriter::visitMetricSet(const MetricSet& set, bool) { - if (!set.getDimensionKey().empty()) { - pushLegacyDimensionFromSet(set); - } else { - pushAllDimensionsFromSet(set); - } + _dimensionStack.push_back(set.getTags()); return true; } diff --git a/metrics/src/vespa/metrics/jsonwriter.h b/metrics/src/vespa/metrics/jsonwriter.h index 07fdc9c22c2..300f0cbb0ec 100644 --- a/metrics/src/vespa/metrics/jsonwriter.h +++ b/metrics/src/vespa/metrics/jsonwriter.h @@ -33,8 +33,6 @@ private: void writeCommonPrefix(const Metric& m); void writeCommonPostfix(const Metric& m); - void pushLegacyDimensionFromSet(const MetricSet&); - void pushAllDimensionsFromSet(const MetricSet&); void writeDimensions(const DimensionSet&); void writeInheritedDimensions(); void writeMetricSpecificDimensions(const Metric&); diff --git a/metrics/src/vespa/metrics/metric.cpp b/metrics/src/vespa/metrics/metric.cpp index f6be5f79357..e67398e4626 100644 --- a/metrics/src/vespa/metrics/metric.cpp +++ b/metrics/src/vespa/metrics/metric.cpp @@ -116,6 +116,7 @@ void Metric::assignMangledNameWithDimensions() { if (!tagsSpecifyAtLeastOneDimension(_tags)) { + _mangledName = _name; return; } sortTagsInDeterministicOrder(); diff --git a/metrics/src/vespa/metrics/metric.h b/metrics/src/vespa/metrics/metric.h index 5b617e68a7f..91ad88fecf7 100644 --- a/metrics/src/vespa/metrics/metric.h +++ b/metrics/src/vespa/metrics/metric.h @@ -215,12 +215,17 @@ public: virtual bool visit(MetricVisitor& visitor, bool tagAsAutoGenerated = false) const = 0; /** Used by sum metric to alter name of cloned metric for sum. */ - void setName(const String& name) { _name = name; } + void setName(const String& name) { + _name = name; + assignMangledNameWithDimensions(); + } + /** Used by sum metric to alter description of cloned metric for sum. */ void setDescription(const String& d) { _description = d; } /** Used by sum metric to alter tag of cloned metric for sum. */ void setTags(Tags tags) { _tags = std::move(tags); + assignMangledNameWithDimensions(); } /** Set whether metrics have ever been set. */ diff --git a/metrics/src/vespa/metrics/metricset.cpp b/metrics/src/vespa/metrics/metricset.cpp index 2e708e17504..cfaeb8b6f02 100644 --- a/metrics/src/vespa/metrics/metricset.cpp +++ b/metrics/src/vespa/metrics/metricset.cpp @@ -15,12 +15,10 @@ LOG_SETUP(".metrics.metricsset"); namespace metrics { MetricSet::MetricSet(const String& name, const String& tags, - const String& description, MetricSet* owner, - const std::string& dimensionKey) + const String& description, MetricSet* owner) : Metric(name, tags, description, owner), _metricOrder(), - _registrationAltered(false), - _dimensionKey(dimensionKey) + _registrationAltered(false) { } @@ -28,8 +26,7 @@ MetricSet::MetricSet(const String& name, Tags dimensions, const String& description, MetricSet* owner) : Metric(name, std::move(dimensions), description, owner), _metricOrder(), - _registrationAltered(false), - _dimensionKey() + _registrationAltered(false) { } @@ -40,13 +37,8 @@ MetricSet::MetricSet(const MetricSet& other, bool includeUnused) : Metric(other, owner), _metricOrder(), - _registrationAltered(false), - _dimensionKey(other._dimensionKey) + _registrationAltered(false) { - if (copyType == INACTIVE && owner == NULL && includeUnused) { - _dimensionKey = ""; - } - for (const Metric* metric : other._metricOrder) { if (copyType != INACTIVE || includeUnused || metric->used()) { Metric* m = metric->clone(ownerList, copyType, this, includeUnused); diff --git a/metrics/src/vespa/metrics/metricset.h b/metrics/src/vespa/metrics/metricset.h index 9671aaffbfc..48027db374a 100644 --- a/metrics/src/vespa/metrics/metricset.h +++ b/metrics/src/vespa/metrics/metricset.h @@ -20,14 +20,10 @@ class MetricSet : public Metric bool _registrationAltered; // Set to true if metrics have been // registered/unregistered since last time // it was reset - std::string _dimensionKey; // If this metric is part of a monitoring dimension, - // the key of the dimension should be set here. - // If so, the name of the metric is used as dimension value. public: MetricSet(const String& name, const String& tags, - const String& description, MetricSet* owner = 0, - const std::string& dimensionKey = ""); + const String& description, MetricSet* owner = 0); MetricSet(const String& name, Tags dimensions, const String& description, MetricSet* owner = 0); @@ -85,11 +81,6 @@ public: bool isMetricSet() const override { return true; } void addToPart(Metric& m) const override { addTo(m, 0); } - /** - * Returns the key of the dimension this metric is part of (if any). - */ - const std::string& getDimensionKey() const { return _dimensionKey; } - private: // Do not generate default copy constructor or assignment operator // These would screw up metric registering 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 3871bb82313..22957124da1 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 @@ -16,7 +16,7 @@ import com.yahoo.vespa.athenz.identityprovider.api.EntityBindingsMapper; import com.yahoo.vespa.athenz.identityprovider.api.IdentityDocumentClient; import com.yahoo.vespa.athenz.identityprovider.api.SignedIdentityDocument; import com.yahoo.vespa.athenz.identityprovider.client.DefaultIdentityDocumentClient; -import com.yahoo.vespa.athenz.identityprovider.client.InstanceCsrGenerator; +import com.yahoo.vespa.athenz.identityprovider.client.CsrGenerator; import com.yahoo.vespa.athenz.tls.AthenzIdentityVerifier; import com.yahoo.vespa.athenz.utils.SiaUtils; import com.yahoo.vespa.hosted.dockerapi.ContainerName; @@ -65,7 +65,7 @@ public class AthenzCredentialsMaintainer { private final Clock clock; private final ServiceIdentityProvider hostIdentityProvider; private final IdentityDocumentClient identityDocumentClient; - private final InstanceCsrGenerator csrGenerator; + private final CsrGenerator csrGenerator; private final AthenzService configserverIdentity; private Instant lastRefreshAttempt = Instant.EPOCH; // Used as an optimization to ensure ZTS is not DDoS'ed on continuously failing refresh attempts @@ -81,7 +81,7 @@ public class AthenzCredentialsMaintainer { this.containerIdentity = environment.getNodeAthenzIdentity(); this.ztsEndpoint = environment.getZtsUri(); this.configserverIdentity = environment.getConfigserverAthenzIdentity(); - this.csrGenerator = new InstanceCsrGenerator(environment.getCertificateDnsSuffix()); + this.csrGenerator = new CsrGenerator(environment.getCertificateDnsSuffix(), configserverIdentity.getFullName()); this.trustStorePath = environment.getTrustStorePath(); this.privateKeyFile = SiaUtils.getPrivateKeyFile(containerSiaDirectory, containerIdentity); this.certificateFile = SiaUtils.getCertificateFile(containerSiaDirectory, containerIdentity); @@ -172,7 +172,7 @@ public class AthenzCredentialsMaintainer { private void registerIdentity() { KeyPair keyPair = KeyUtils.generateKeypair(KeyAlgorithm.RSA); SignedIdentityDocument signedIdentityDocument = identityDocumentClient.getNodeIdentityDocument(hostname); - com.yahoo.vespa.athenz.tls.Pkcs10Csr csr = csrGenerator.generateCsr( + com.yahoo.vespa.athenz.tls.Pkcs10Csr csr = csrGenerator.generateInstanceCsr( containerIdentity, signedIdentityDocument.providerUniqueId(), signedIdentityDocument.ipAddresses(), keyPair); try (ZtsClient ztsClient = new DefaultZtsClient(ztsEndpoint, hostIdentityProvider)) { InstanceIdentity instanceIdentity = @@ -195,7 +195,7 @@ public class AthenzCredentialsMaintainer { private void refreshIdentity() { SignedIdentityDocument identityDocument = EntityBindingsMapper.readSignedIdentityDocumentFromFile(identityDocumentFile); KeyPair keyPair = KeyUtils.generateKeypair(KeyAlgorithm.RSA); - com.yahoo.vespa.athenz.tls.Pkcs10Csr csr = csrGenerator.generateCsr(containerIdentity, identityDocument.providerUniqueId(), identityDocument.ipAddresses(), keyPair); + com.yahoo.vespa.athenz.tls.Pkcs10Csr csr = csrGenerator.generateInstanceCsr(containerIdentity, identityDocument.providerUniqueId(), identityDocument.ipAddresses(), keyPair); SSLContext containerIdentitySslContext = new SslContextBuilder() .withKeyStore(privateKeyFile, certificateFile) diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java index e03250e7934..cef913b18ae 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java @@ -214,7 +214,9 @@ public class NodeRepository extends AbstractComponent { break; case proxy: - // Accept connections from the world on 4443 + // Accept connections from the world on 443 (for dashboard app), 4080 (insecure tb removed), and 4443 + trustedPorts.add(443); + trustedPorts.add(4080); trustedPorts.add(4443); break; diff --git a/storage/src/tests/visiting/visitortest.cpp b/storage/src/tests/visiting/visitortest.cpp index 4fc577226ca..11e5e355f0c 100644 --- a/storage/src/tests/visiting/visitortest.cpp +++ b/storage/src/tests/visiting/visitortest.cpp @@ -119,6 +119,8 @@ public: } }; + constexpr static api::StorageMessage::Priority DefaultPriority = 123; + std::shared_ptr<api::CreateVisitorCommand> makeCreateVisitor( const VisitorOptions& options = VisitorOptions()); void tearDown() override; @@ -495,6 +497,7 @@ VisitorTest::makeCreateVisitor(const VisitorOptions& options) cmd->setAddress(address); cmd->setMaximumPendingReplyCount(UINT32_MAX); cmd->setControlDestination("foo/bar"); + cmd->setPriority(DefaultPriority); return cmd; } @@ -519,7 +522,8 @@ VisitorTest::testNormalUsage() CreateIteratorCommand::SP createCmd( fetchSingleCommand<CreateIteratorCommand>(*_bottom)); - CPPUNIT_ASSERT_EQUAL(uint8_t(0), createCmd->getPriority()); // Highest pri + CPPUNIT_ASSERT_EQUAL(static_cast<int>(DefaultPriority), + static_cast<int>(createCmd->getPriority())); // Inherit pri spi::IteratorId id(1234); api::StorageReply::SP reply( new CreateIteratorReply(*createCmd, id)); diff --git a/storage/src/vespa/storage/bucketdb/bucketmanagermetrics.cpp b/storage/src/vespa/storage/bucketdb/bucketmanagermetrics.cpp index 84b90ceedec..b95fb6b814c 100644 --- a/storage/src/vespa/storage/bucketdb/bucketmanagermetrics.cpp +++ b/storage/src/vespa/storage/bucketdb/bucketmanagermetrics.cpp @@ -10,7 +10,7 @@ using vespalib::IllegalStateException; using vespalib::make_string; DataStoredMetrics::DataStoredMetrics(const std::string& name, metrics::MetricSet* owner) - : metrics::MetricSet(name, "partofsum yamasdefault", "", owner, "disk"), + : metrics::MetricSet(name, "partofsum yamasdefault", "", owner), buckets("buckets", "", "buckets managed", this), docs("docs", "", "documents stored", this), bytes("bytes", "", "bytes stored", this), diff --git a/storage/src/vespa/storage/distributor/persistence_operation_metric_set.cpp b/storage/src/vespa/storage/distributor/persistence_operation_metric_set.cpp index d5f73a9e916..30fa2797ef4 100644 --- a/storage/src/vespa/storage/distributor/persistence_operation_metric_set.cpp +++ b/storage/src/vespa/storage/distributor/persistence_operation_metric_set.cpp @@ -55,7 +55,7 @@ PersistenceFailuresMetricSet::clone(std::vector<Metric::UP>& ownerList, CopyType } PersistenceOperationMetricSet::PersistenceOperationMetricSet(const std::string& name, MetricSet* owner) - : MetricSet(name, "", vespalib::make_string("Statistics for the %s command", name.c_str()), owner, "operationtype"), + : MetricSet(name, "", vespalib::make_string("Statistics for the %s command", name.c_str()), owner), latency("latency", "yamasdefault", vespalib::make_string("The average latency of %s operations", name.c_str()), this), ok("ok", "logdefault yamasdefault", vespalib::make_string("The number of successful %s operations performed", name.c_str()), this), failures(this) diff --git a/storage/src/vespa/storage/persistence/filestorage/filestormetrics.cpp b/storage/src/vespa/storage/persistence/filestorage/filestormetrics.cpp index 1293b5d1808..52d95e9a3ed 100644 --- a/storage/src/vespa/storage/persistence/filestorage/filestormetrics.cpp +++ b/storage/src/vespa/storage/persistence/filestorage/filestormetrics.cpp @@ -10,7 +10,7 @@ using metrics::MetricSet; using metrics::LoadTypeSet; FileStorThreadMetrics::Op::Op(const std::string& id, const std::string& name, MetricSet* owner) - : MetricSet(id, id, name + " load in filestor thread", owner, "operationtype"), + : MetricSet(id, "", name + " load in filestor thread", owner), _name(name), count("count", "yamasdefault", "Number of requests processed.", this), latency("latency", "yamasdefault", "Latency of successful requests.", this), @@ -118,7 +118,7 @@ FileStorThreadMetrics::Visitor::clone(std::vector<Metric::UP>& ownerList, } FileStorThreadMetrics::FileStorThreadMetrics(const std::string& name, const std::string& desc, const LoadTypeSet& lt) - : MetricSet(name, "filestor partofsum thread", desc, nullptr, "thread"), + : MetricSet(name, "filestor partofsum", desc), operations("operations", "", "Number of operations processed.", this), failedOperations("failedoperations", "", "Number of operations throwing exceptions.", this), put(lt, OpWithRequestSize<Op>("put", "Put"), this), @@ -176,7 +176,7 @@ FileStorThreadMetrics::~FileStorThreadMetrics() = default; FileStorStripeMetrics::FileStorStripeMetrics(const std::string& name, const std::string& description, const LoadTypeSet& loadTypes) - : MetricSet(name, "partofsum stripe", description, nullptr, "stripe"), + : MetricSet(name, "partofsum", description), averageQueueWaitingTime(loadTypes, metrics::DoubleAverageMetric("averagequeuewait", "", "Average time an operation spends in input queue."), @@ -188,7 +188,7 @@ FileStorStripeMetrics::~FileStorStripeMetrics() = default; FileStorDiskMetrics::FileStorDiskMetrics(const std::string& name, const std::string& description, const metrics::LoadTypeSet& loadTypes, MetricSet* owner) - : MetricSet(name, "partofsum disk", description, owner, "disk"), + : MetricSet(name, "partofsum", description, owner), sumThreads("allthreads", "sum", "", this), sumStripes("allstripes", "sum", "", this), averageQueueWaitingTime(loadTypes, diff --git a/storage/src/vespa/storage/visiting/visitor.cpp b/storage/src/vespa/storage/visiting/visitor.cpp index 2f028b5c9fa..60b94b6b33e 100644 --- a/storage/src/vespa/storage/visiting/visitor.cpp +++ b/storage/src/vespa/storage/visiting/visitor.cpp @@ -1257,7 +1257,7 @@ Visitor::getIterators() cmd->setLoadType(_initiatingCmd->getLoadType()); cmd->getTrace().setLevel(_traceLevel); - cmd->setPriority(0); + cmd->setPriority(_initiatingCmd->getPriority()); cmd->setReadConsistency(getRequiredReadConsistency()); _bucketStates.push_back(newBucketState.release()); _messageHandler->send(cmd, *this); diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zts/DefaultZtsClient.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zts/DefaultZtsClient.java index 7d4901f163a..dc82ed7fcb9 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zts/DefaultZtsClient.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zts/DefaultZtsClient.java @@ -24,7 +24,6 @@ import com.yahoo.vespa.athenz.client.zts.utils.IdentityCsrGenerator; import com.yahoo.vespa.athenz.identity.ServiceIdentityProvider; import com.yahoo.vespa.athenz.identity.ServiceIdentitySslSocketFactory; import com.yahoo.vespa.athenz.tls.Pkcs10Csr; -import com.yahoo.vespa.athenz.tls.Pkcs10CsrBuilder; import org.apache.http.HttpResponse; import org.apache.http.client.ResponseHandler; import org.apache.http.client.config.RequestConfig; @@ -40,7 +39,6 @@ import org.eclipse.jetty.http.HttpStatus; import javax.net.ssl.HostnameVerifier; import javax.net.ssl.SSLContext; -import javax.security.auth.x500.X500Principal; import java.io.IOException; import java.io.UncheckedIOException; import java.net.URI; @@ -50,9 +48,6 @@ import java.time.Duration; import java.util.List; import java.util.function.Supplier; -import static com.yahoo.vespa.athenz.tls.SignatureAlgorithm.SHA256_WITH_RSA; -import static com.yahoo.vespa.athenz.tls.SubjectAlternativeName.Type.DNS_NAME; -import static com.yahoo.vespa.athenz.tls.SubjectAlternativeName.Type.RFC822_NAME; import static java.util.stream.Collectors.toList; /** @@ -163,15 +158,7 @@ public class DefaultZtsClient implements ZtsClient { } @Override - public X509Certificate getRoleCertificate(AthenzRole role, - Duration expiry, - KeyPair keyPair, - String cloud) { - X500Principal principal = new X500Principal(String.format("cn=%s:role.%s", role.domain().getName(), role.roleName())); - Pkcs10Csr csr = Pkcs10CsrBuilder.fromKeypair(principal, keyPair, SHA256_WITH_RSA) - .addSubjectAlternativeName(DNS_NAME, String.format("%s.%s.%s", identity.getName(), identity.getDomainName().replace('.', '-'), cloud)) - .addSubjectAlternativeName(RFC822_NAME, String.format("%s.%s@%s", identity.getDomainName(), identity.getName(), cloud)) - .build(); + public X509Certificate getRoleCertificate(AthenzRole role, Pkcs10Csr csr, Duration expiry) { RoleCertificateRequestEntity requestEntity = new RoleCertificateRequestEntity(csr, expiry); URI uri = ztsUrl.resolve(String.format("domain/%s/role/%s/token", role.domain().getName(), role.roleName())); HttpUriRequest request = RequestBuilder.post(uri) @@ -184,10 +171,8 @@ public class DefaultZtsClient implements ZtsClient { } @Override - public X509Certificate getRoleCertificate(AthenzRole role, - KeyPair keyPair, - String cloud) { - return getRoleCertificate(role, null, keyPair, cloud); + public X509Certificate getRoleCertificate(AthenzRole role, Pkcs10Csr csr) { + return getRoleCertificate(role, csr, null); } @Override diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zts/ZtsClient.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zts/ZtsClient.java index 5c0e21bfa97..2ef6039ddc8 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zts/ZtsClient.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/client/zts/ZtsClient.java @@ -1,7 +1,6 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.athenz.client.zts; -import com.yahoo.athenz.zts.TenantDomains; import com.yahoo.vespa.athenz.api.AthenzDomain; import com.yahoo.vespa.athenz.api.AthenzIdentity; import com.yahoo.vespa.athenz.api.AthenzRole; @@ -84,27 +83,20 @@ public interface ZtsClient extends AutoCloseable { * Fetch role certificate for the target domain and role * * @param role Target role + * @param csr Certificate signing request matching role * @param expiry Certificate expiry - * @param keyPair Key pair which will be used to generate CSR (certificate signing request) - * @param cloud The cloud suffix used in DNS SAN entries * @return A role certificate */ - X509Certificate getRoleCertificate(AthenzRole role, - Duration expiry, - KeyPair keyPair, - String cloud); + X509Certificate getRoleCertificate(AthenzRole role, Pkcs10Csr csr, Duration expiry); /** * Fetch role certificate for the target domain and role * * @param role Target role - * @param keyPair Key pair which will be used to generate CSR (certificate signing request) - * @param cloud The cloud suffix used in DNS SAN entries + * @param csr Certificate signing request matching role * @return A role certificate */ - X509Certificate getRoleCertificate(AthenzRole role, - KeyPair keyPair, - String cloud); + X509Certificate getRoleCertificate(AthenzRole role, Pkcs10Csr csr); /** * For a given provider, get a list of tenant domains that the user is a member of diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzCredentialsService.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzCredentialsService.java index 4a189c872bc..907c262e4d3 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzCredentialsService.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzCredentialsService.java @@ -2,6 +2,9 @@ package com.yahoo.vespa.athenz.identityprovider.client; import com.yahoo.container.core.identity.IdentityConfig; +import com.yahoo.security.KeyAlgorithm; +import com.yahoo.security.KeyUtils; +import com.yahoo.security.SslContextBuilder; import com.yahoo.vespa.athenz.api.AthenzService; import com.yahoo.vespa.athenz.client.zts.DefaultZtsClient; import com.yahoo.vespa.athenz.client.zts.InstanceIdentity; @@ -11,9 +14,6 @@ import com.yahoo.vespa.athenz.identityprovider.api.EntityBindingsMapper; import com.yahoo.vespa.athenz.identityprovider.api.IdentityDocumentClient; import com.yahoo.vespa.athenz.identityprovider.api.SignedIdentityDocument; import com.yahoo.vespa.athenz.tls.AthenzIdentityVerifier; -import com.yahoo.security.KeyAlgorithm; -import com.yahoo.security.KeyUtils; -import com.yahoo.security.SslContextBuilder; import com.yahoo.vespa.athenz.tls.Pkcs10Csr; import com.yahoo.vespa.athenz.utils.SiaUtils; import com.yahoo.vespa.defaults.Defaults; @@ -51,7 +51,7 @@ class AthenzCredentialsService { private final ServiceIdentityProvider nodeIdentityProvider; private final File trustStoreJks; private final String hostname; - private final InstanceCsrGenerator instanceCsrGenerator; + private final CsrGenerator csrGenerator; private final Clock clock; AthenzCredentialsService(IdentityConfig identityConfig, @@ -66,7 +66,7 @@ class AthenzCredentialsService { this.nodeIdentityProvider = nodeIdentityProvider; this.trustStoreJks = trustStoreJks; this.hostname = hostname; - this.instanceCsrGenerator = new InstanceCsrGenerator(identityConfig.athenzDnsSuffix()); + this.csrGenerator = new CsrGenerator(identityConfig.athenzDnsSuffix(), identityConfig.configserverIdentityName()); this.clock = clock; } @@ -78,7 +78,7 @@ class AthenzCredentialsService { KeyPair keyPair = KeyUtils.generateKeypair(KeyAlgorithm.RSA); IdentityDocumentClient identityDocumentClient = createIdentityDocumentClient(); SignedIdentityDocument document = identityDocumentClient.getTenantIdentityDocument(hostname); - Pkcs10Csr csr = instanceCsrGenerator.generateCsr( + Pkcs10Csr csr = csrGenerator.generateInstanceCsr( tenantIdentity, document.providerUniqueId(), document.ipAddresses(), @@ -102,7 +102,7 @@ class AthenzCredentialsService { AthenzCredentials updateCredentials(SignedIdentityDocument document, SSLContext sslContext) { KeyPair newKeyPair = KeyUtils.generateKeypair(KeyAlgorithm.RSA); - Pkcs10Csr csr = instanceCsrGenerator.generateCsr( + Pkcs10Csr csr = csrGenerator.generateInstanceCsr( tenantIdentity, document.providerUniqueId(), document.ipAddresses(), diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzIdentityProviderImpl.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzIdentityProviderImpl.java index 9b4bdd35e8e..333f5ec9b85 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzIdentityProviderImpl.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/AthenzIdentityProviderImpl.java @@ -22,6 +22,7 @@ import com.yahoo.vespa.athenz.identity.ServiceIdentityProvider; import com.yahoo.vespa.athenz.identity.SiaIdentityProvider; import com.yahoo.security.KeyStoreType; import com.yahoo.security.SslContextBuilder; +import com.yahoo.vespa.athenz.tls.Pkcs10Csr; import com.yahoo.vespa.athenz.utils.SiaUtils; import com.yahoo.vespa.defaults.Defaults; @@ -69,6 +70,7 @@ public final class AthenzIdentityProviderImpl extends AbstractComponent implemen private final LoadingCache<AthenzRole, SSLContext> roleSslContextCache; private final LoadingCache<AthenzRole, ZToken> roleSpecificRoleTokenCache; private final LoadingCache<AthenzDomain, ZToken> domainSpecificRoleTokenCache; + private final CsrGenerator csrGenerator; @Inject public AthenzIdentityProviderImpl(IdentityConfig config, Metric metric) { @@ -100,6 +102,7 @@ public final class AthenzIdentityProviderImpl extends AbstractComponent implemen roleSslContextCache = createCache(ROLE_SSL_CONTEXT_EXPIRY, this::createRoleSslContext); roleSpecificRoleTokenCache = createCache(ROLE_TOKEN_EXPIRY, this::createRoleToken); domainSpecificRoleTokenCache = createCache(ROLE_TOKEN_EXPIRY, this::createRoleToken); + this.csrGenerator = new CsrGenerator(config.athenzDnsSuffix(), config.configserverIdentityName()); registerInstance(); } @@ -174,8 +177,9 @@ public final class AthenzIdentityProviderImpl extends AbstractComponent implemen } private SSLContext createRoleSslContext(AthenzRole role) { + Pkcs10Csr csr = csrGenerator.generateRoleCsr(identity, role, credentials.getIdentityDocument().providerUniqueId(), credentials.getKeyPair()); try (ZtsClient client = createZtsClient()) { - X509Certificate roleCertificate = client.getRoleCertificate(role, credentials.getKeyPair(), dnsSuffix); + X509Certificate roleCertificate = client.getRoleCertificate(role, csr); return new SslContextBuilder() .withKeyStore(credentials.getKeyPair().getPrivate(), roleCertificate) .withTrustStore(getDefaultTrustStoreLocation().toPath(), KeyStoreType.JKS) diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/CsrGenerator.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/CsrGenerator.java new file mode 100644 index 00000000000..e22c8621e99 --- /dev/null +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/CsrGenerator.java @@ -0,0 +1,69 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.athenz.identityprovider.client; + +import com.yahoo.vespa.athenz.api.AthenzIdentity; +import com.yahoo.vespa.athenz.api.AthenzRole; +import com.yahoo.vespa.athenz.identityprovider.api.VespaUniqueInstanceId; +import com.yahoo.vespa.athenz.tls.Pkcs10Csr; +import com.yahoo.vespa.athenz.tls.Pkcs10CsrBuilder; +import com.yahoo.vespa.athenz.tls.SubjectAlternativeName; + +import javax.security.auth.x500.X500Principal; +import java.security.KeyPair; +import java.util.Set; + +import static com.yahoo.vespa.athenz.tls.SignatureAlgorithm.SHA256_WITH_RSA; +import static com.yahoo.vespa.athenz.tls.SubjectAlternativeName.Type.DNS_NAME; +import static com.yahoo.vespa.athenz.tls.SubjectAlternativeName.Type.IP_ADDRESS; +import static com.yahoo.vespa.athenz.tls.SubjectAlternativeName.Type.RFC822_NAME; + +/** + * Generates a {@link Pkcs10Csr} for an instance. + * + * @author bjorncs + */ +public class CsrGenerator { + + private final String dnsSuffix; + private final String providerService; + + public CsrGenerator(String dnsSuffix, String providerService) { + this.dnsSuffix = dnsSuffix; + this.providerService = providerService; + } + + public Pkcs10Csr generateInstanceCsr(AthenzIdentity instanceIdentity, + VespaUniqueInstanceId instanceId, + Set<String> ipAddresses, + KeyPair keyPair) { + X500Principal subject = new X500Principal(String.format("OU=%s, CN=%s", providerService, instanceIdentity.getFullName())); + // Add SAN dnsname <service>.<domain-with-dashes>.<provider-dnsname-suffix> + // and SAN dnsname <provider-unique-instance-id>.instanceid.athenz.<provider-dnsname-suffix> + Pkcs10CsrBuilder pkcs10CsrBuilder = Pkcs10CsrBuilder.fromKeypair(subject, keyPair, SHA256_WITH_RSA) + .addSubjectAlternativeName( + DNS_NAME, + String.format( + "%s.%s.%s", + instanceIdentity.getName(), + instanceIdentity.getDomainName().replace(".", "-"), + dnsSuffix)) + .addSubjectAlternativeName(DNS_NAME, getIdentitySAN(instanceId)); + ipAddresses.forEach(ip -> pkcs10CsrBuilder.addSubjectAlternativeName(new SubjectAlternativeName(IP_ADDRESS, ip))); + return pkcs10CsrBuilder.build(); + } + + public Pkcs10Csr generateRoleCsr(AthenzIdentity identity, + AthenzRole role, + VespaUniqueInstanceId instanceId, + KeyPair keyPair) { + X500Principal principal = new X500Principal(String.format("OU=%s, cn=%s:role.%s", providerService, role.domain().getName(), role.roleName())); + return Pkcs10CsrBuilder.fromKeypair(principal, keyPair, SHA256_WITH_RSA) + .addSubjectAlternativeName(DNS_NAME, getIdentitySAN(instanceId)) + .addSubjectAlternativeName(RFC822_NAME, String.format("%s.%s@%s", identity.getDomainName(), identity.getName(), dnsSuffix)) + .build(); + } + + private String getIdentitySAN(VespaUniqueInstanceId instanceId) { + return String.format("%s.instanceid.athenz.%s", instanceId.asDottedString(), dnsSuffix); + } +} diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/InstanceCsrGenerator.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/InstanceCsrGenerator.java deleted file mode 100644 index 70227eae91c..00000000000 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/identityprovider/client/InstanceCsrGenerator.java +++ /dev/null @@ -1,50 +0,0 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.athenz.identityprovider.client; - -import com.yahoo.vespa.athenz.api.AthenzIdentity; -import com.yahoo.vespa.athenz.identityprovider.api.VespaUniqueInstanceId; -import com.yahoo.vespa.athenz.tls.Pkcs10Csr; -import com.yahoo.vespa.athenz.tls.Pkcs10CsrBuilder; -import com.yahoo.vespa.athenz.tls.SignatureAlgorithm; -import com.yahoo.vespa.athenz.tls.SubjectAlternativeName; - -import javax.security.auth.x500.X500Principal; -import java.security.KeyPair; -import java.util.Set; - -import static com.yahoo.vespa.athenz.tls.SubjectAlternativeName.Type.DNS_NAME; -import static com.yahoo.vespa.athenz.tls.SubjectAlternativeName.Type.IP_ADDRESS; - -/** - * Generates a {@link Pkcs10Csr} for an instance. - * - * @author bjorncs - */ -public class InstanceCsrGenerator { - - private final String dnsSuffix; - - public InstanceCsrGenerator(String dnsSuffix) { - this.dnsSuffix = dnsSuffix; - } - - public Pkcs10Csr generateCsr(AthenzIdentity instanceIdentity, - VespaUniqueInstanceId instanceId, - Set<String> ipAddresses, - KeyPair keyPair) { - X500Principal subject = new X500Principal("CN=" + instanceIdentity.getFullName()); - // Add SAN dnsname <service>.<domain-with-dashes>.<provider-dnsname-suffix> - // and SAN dnsname <provider-unique-instance-id>.instanceid.athenz.<provider-dnsname-suffix> - Pkcs10CsrBuilder pkcs10CsrBuilder = Pkcs10CsrBuilder.fromKeypair(subject, keyPair, SignatureAlgorithm.SHA256_WITH_RSA) - .addSubjectAlternativeName( - DNS_NAME, - String.format( - "%s.%s.%s", - instanceIdentity.getName(), - instanceIdentity.getDomainName().replace(".", "-"), - dnsSuffix)) - .addSubjectAlternativeName(DNS_NAME, String.format("%s.instanceid.athenz.%s", instanceId.asDottedString(), dnsSuffix)); - ipAddresses.forEach(ip -> pkcs10CsrBuilder.addSubjectAlternativeName(new SubjectAlternativeName(IP_ADDRESS, ip))); - return pkcs10CsrBuilder.build(); - } -} diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/tls/Pkcs10CsrBuilder.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/tls/Pkcs10CsrBuilder.java index 702b2f6cd4b..607bec90dee 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/tls/Pkcs10CsrBuilder.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/tls/Pkcs10CsrBuilder.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.athenz.tls; import org.bouncycastle.asn1.pkcs.PKCSObjectIdentifiers; +import org.bouncycastle.asn1.x500.X500Name; import org.bouncycastle.asn1.x509.BasicConstraints; import org.bouncycastle.asn1.x509.Extension; import org.bouncycastle.asn1.x509.ExtensionsGenerator; @@ -72,7 +73,7 @@ public class Pkcs10CsrBuilder { public Pkcs10Csr build() { try { PKCS10CertificationRequestBuilder requestBuilder = - new JcaPKCS10CertificationRequestBuilder(subject, keyPair.getPublic()); + new JcaPKCS10CertificationRequestBuilder(new X500Name(subject.getName()), keyPair.getPublic()); ExtensionsGenerator extGen = new ExtensionsGenerator(); if (basicConstraintsExtension != null) { extGen.addExtension( diff --git a/vespa-athenz/src/test/java/com/yahoo/vespa/athenz/identityprovider/client/InstanceCsrGeneratorTest.java b/vespa-athenz/src/test/java/com/yahoo/vespa/athenz/identityprovider/client/InstanceCsrGeneratorTest.java new file mode 100644 index 00000000000..ed5c5586d6d --- /dev/null +++ b/vespa-athenz/src/test/java/com/yahoo/vespa/athenz/identityprovider/client/InstanceCsrGeneratorTest.java @@ -0,0 +1,37 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.athenz.identityprovider.client; + +import com.yahoo.security.KeyAlgorithm; +import com.yahoo.security.KeyUtils; +import com.yahoo.vespa.athenz.api.AthenzService; +import com.yahoo.vespa.athenz.identityprovider.api.VespaUniqueInstanceId; +import com.yahoo.vespa.athenz.tls.Pkcs10Csr; +import org.junit.Test; + +import javax.security.auth.x500.X500Principal; +import java.security.KeyPair; +import java.util.Collections; + +import static org.junit.Assert.assertEquals; + +/** + * @author mortent + */ +public class InstanceCsrGeneratorTest { + + private static final String DNS_SUFFIX = "prod-us-north-1.vespa.yahoo.cloud"; + private static final String PROVIDER_SERVICE = "vespa.vespa.provider_prod_us-north-1"; + private static final String ATHENZ_SERVICE = "foo.bar"; + + @Test + public void it_generates_csr_with_correct_subject() { + CsrGenerator csrGenerator = new CsrGenerator(DNS_SUFFIX, PROVIDER_SERVICE); + + AthenzService service = new AthenzService(ATHENZ_SERVICE); + VespaUniqueInstanceId vespaUniqueInstanceId = VespaUniqueInstanceId.fromDottedString("0.default.default.foo-app.vespa.us-north-1.prod.node"); + KeyPair keyPair = KeyUtils.generateKeypair(KeyAlgorithm.RSA); + + Pkcs10Csr csr = csrGenerator.generateInstanceCsr(service, vespaUniqueInstanceId, Collections.emptySet(), keyPair); + assertEquals(new X500Principal(String.format("OU=%s, CN=%s", PROVIDER_SERVICE, ATHENZ_SERVICE)), csr.getSubject()); + } +} |