diff options
98 files changed, 2066 insertions, 663 deletions
diff --git a/client/go/.gitignore b/client/go/.gitignore index eb679add05e..8933bc220cb 100644 --- a/client/go/.gitignore +++ b/client/go/.gitignore @@ -4,3 +4,5 @@ share/ !Makefile !build/ !target/ +mytestapp/ +mytestapp-cache/ diff --git a/client/go/cmd/api_key.go b/client/go/cmd/api_key.go index 5cc1dab8a35..ae3f5346f4c 100644 --- a/client/go/cmd/api_key.go +++ b/client/go/cmd/api_key.go @@ -79,11 +79,19 @@ func doApiKey(_ *cobra.Command, _ []string) error { if err != nil { return err } + targetType, err := getTargetType() + if err != nil { + return err + } + system, err := getSystem(targetType) + if err != nil { + return err + } apiKeyFile := cfg.APIKeyPath(app.Tenant) if util.PathExists(apiKeyFile) && !overwriteKey { err := fmt.Errorf("refusing to overwrite %s", apiKeyFile) printErrHint(err, "Use -f to overwrite it") - printPublicKey(apiKeyFile, app.Tenant) + printPublicKey(system, apiKeyFile, app.Tenant) return ErrCLI{error: err, quiet: true} } apiKey, err := vespa.CreateAPIKey() @@ -92,13 +100,13 @@ func doApiKey(_ *cobra.Command, _ []string) error { } if err := ioutil.WriteFile(apiKeyFile, apiKey, 0600); err == nil { printSuccess("API private key written to ", apiKeyFile) - return printPublicKey(apiKeyFile, app.Tenant) + return printPublicKey(system, apiKeyFile, app.Tenant) } else { return fmt.Errorf("failed to write: %s: %w", apiKeyFile, err) } } -func printPublicKey(apiKeyFile, tenant string) error { +func printPublicKey(system vespa.System, apiKeyFile, tenant string) error { pemKeyData, err := ioutil.ReadFile(apiKeyFile) if err != nil { return fmt.Errorf("failed to read: %s: %w", apiKeyFile, err) @@ -118,7 +126,7 @@ func printPublicKey(apiKeyFile, tenant string) error { log.Printf("\nThis is your public key:\n%s", color.Green(pemPublicKey)) log.Printf("Its fingerprint is:\n%s\n", color.Cyan(fingerprint)) log.Print("\nTo use this key in Vespa Cloud click 'Add custom key' at") - log.Printf(color.Cyan("%s/tenant/%s/keys").String(), getConsoleURL(), tenant) + log.Printf(color.Cyan("%s/tenant/%s/keys").String(), system.ConsoleURL, tenant) log.Print("and paste the entire public key including the BEGIN and END lines.") return nil } diff --git a/client/go/cmd/api_key_test.go b/client/go/cmd/api_key_test.go index 935b8676c09..ba697b69d9f 100644 --- a/client/go/cmd/api_key_test.go +++ b/client/go/cmd/api_key_test.go @@ -23,6 +23,8 @@ func testAPIKey(t *testing.T, subcommand []string) { homeDir := filepath.Join(t.TempDir(), ".vespa") keyFile := filepath.Join(homeDir, "t1.api-key.pem") + execute(command{args: []string{"config", "set", "target", "cloud"}, homeDir: homeDir}, t, nil) + args := append(subcommand, "-a", "t1.a1.i1") out, _ := execute(command{args: args, homeDir: homeDir}, t, nil) assert.Contains(t, out, "Success: API private key written to "+keyFile+"\n") diff --git a/client/go/cmd/clone_list_test.go b/client/go/cmd/clone_list_test.go index 1138e5de064..2e4fc4004bd 100644 --- a/client/go/cmd/clone_list_test.go +++ b/client/go/cmd/clone_list_test.go @@ -7,11 +7,12 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/vespa-engine/vespa/client/go/mock" "github.com/vespa-engine/vespa/client/go/util" ) func TestListSampleApps(t *testing.T) { - c := &mockHttpClient{} + c := &mock.HTTPClient{} c.NextResponse(200, readTestData(t, "sample-apps-contents.json")) c.NextResponse(200, readTestData(t, "sample-apps-news.json")) c.NextResponse(200, readTestData(t, "sample-apps-operations.json")) diff --git a/client/go/cmd/clone_test.go b/client/go/cmd/clone_test.go index 18354ece0e1..332758a127a 100644 --- a/client/go/cmd/clone_test.go +++ b/client/go/cmd/clone_test.go @@ -12,6 +12,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/vespa-engine/vespa/client/go/mock" "github.com/vespa-engine/vespa/client/go/util" ) @@ -21,7 +22,7 @@ func TestClone(t *testing.T) { func assertCreated(sampleAppName string, app string, t *testing.T) { appCached := app + "-cache" - httpClient := &mockHttpClient{} + httpClient := &mock.HTTPClient{} testdata, err := ioutil.ReadFile(filepath.Join("testdata", "sample-apps-master.zip")) require.Nil(t, err) httpClient.NextResponseBytes(200, testdata) diff --git a/client/go/cmd/command_tester.go b/client/go/cmd/command_tester_test.go index 2391cb82d15..d71cdde0e8f 100644 --- a/client/go/cmd/command_tester.go +++ b/client/go/cmd/command_tester_test.go @@ -6,19 +6,15 @@ package cmd import ( "bytes" - "crypto/tls" "io" - "io/ioutil" - "net/http" "os" "path/filepath" - "strconv" "testing" - "time" "github.com/spf13/pflag" "github.com/spf13/viper" "github.com/stretchr/testify/require" + "github.com/vespa-engine/vespa/client/go/mock" "github.com/vespa-engine/vespa/client/go/util" ) @@ -66,7 +62,7 @@ func resetEnv(env map[string]string, original map[string]string) { } } -func execute(cmd command, t *testing.T, client *mockHttpClient) (string, string) { +func execute(cmd command, t *testing.T, client *mock.HTTPClient) (string, string) { if client != nil { util.ActiveHttpClient = client } @@ -122,52 +118,7 @@ func execute(cmd command, t *testing.T, client *mockHttpClient) (string, string) return capturedOut.String(), capturedErr.String() } -func executeCommand(t *testing.T, client *mockHttpClient, args []string, moreArgs []string) string { +func executeCommand(t *testing.T, client *mock.HTTPClient, args []string, moreArgs []string) string { out, _ := execute(command{args: args, moreArgs: moreArgs}, t, client) return out } - -type mockHttpClient struct { - // The responses to return for future requests. Once a response is consumed, it's removed from this array - nextResponses []mockResponse - - // A recording of the last HTTP request made through this - lastRequest *http.Request - - // All requests made through this - requests []*http.Request -} - -type mockResponse struct { - status int - body []byte -} - -func (c *mockHttpClient) NextStatus(status int) { c.NextResponseBytes(status, nil) } - -func (c *mockHttpClient) NextResponse(status int, body string) { - c.NextResponseBytes(status, []byte(body)) -} - -func (c *mockHttpClient) NextResponseBytes(status int, body []byte) { - c.nextResponses = append(c.nextResponses, mockResponse{status: status, body: body}) -} - -func (c *mockHttpClient) Do(request *http.Request, timeout time.Duration) (*http.Response, error) { - response := mockResponse{status: 200} - if len(c.nextResponses) > 0 { - response = c.nextResponses[0] - c.nextResponses = c.nextResponses[1:] - } - c.lastRequest = request - c.requests = append(c.requests, request) - return &http.Response{ - Status: "Status " + strconv.Itoa(response.status), - StatusCode: response.status, - Body: ioutil.NopCloser(bytes.NewBuffer(response.body)), - Header: make(http.Header), - }, - nil -} - -func (c *mockHttpClient) UseCertificate(certificates []tls.Certificate) {} diff --git a/client/go/cmd/config.go b/client/go/cmd/config.go index 9d42f0683fd..0997be2c899 100644 --- a/client/go/cmd/config.go +++ b/client/go/cmd/config.go @@ -197,7 +197,7 @@ func (c *Config) ReadAPIKey(tenantName string) ([]byte, error) { } // UseAPIKey checks if api key should be used be checking if api-key or api-key-file has been set. -func (c *Config) UseAPIKey(tenantName string) bool { +func (c *Config) UseAPIKey(system vespa.System, tenantName string) bool { if _, err := c.Get(apiKeyFlag); err == nil { return true } @@ -207,15 +207,11 @@ func (c *Config) UseAPIKey(tenantName string) bool { // If no Auth0 token is created, fall back to tenant api key, but warn that this functionality is deprecated // TODO: Remove this when users have had time to migrate over to Auth0 device flow authentication - a, err := auth0.GetAuth0(c.AuthConfigPath(), getSystemName(), getApiURL()) + a, err := auth0.GetAuth0(c.AuthConfigPath(), system.Name, system.URL) if err != nil || !a.HasSystem() { fmt.Fprintln(stderr, "Defaulting to tenant API key is deprecated. Use Auth0 device flow: 'vespa auth login' instead") - if !util.PathExists(c.APIKeyPath(tenantName)) { - return false - } - return true + return util.PathExists(c.APIKeyPath(tenantName)) } - return false } @@ -283,7 +279,7 @@ func (c *Config) Set(option, value string) error { switch option { case targetFlag: switch value { - case "local", "cloud": + case vespa.TargetLocal, vespa.TargetCloud, vespa.TargetHosted: viper.Set(option, value) return nil } diff --git a/client/go/cmd/config_test.go b/client/go/cmd/config_test.go index 16378b5f8ba..2f0ccbb29e1 100644 --- a/client/go/cmd/config_test.go +++ b/client/go/cmd/config_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/vespa-engine/vespa/client/go/vespa" ) func TestConfig(t *testing.T) { @@ -16,6 +17,8 @@ func TestConfig(t *testing.T) { assertConfigCommandErr(t, "Error: invalid option or value: \"foo\": \"bar\"\n", homeDir, "config", "set", "foo", "bar") assertConfigCommand(t, "foo = <unset>\n", homeDir, "config", "get", "foo") assertConfigCommand(t, "target = local\n", homeDir, "config", "get", "target") + assertConfigCommand(t, "", homeDir, "config", "set", "target", "hosted") + assertConfigCommand(t, "target = hosted\n", homeDir, "config", "get", "target") assertConfigCommand(t, "", homeDir, "config", "set", "target", "cloud") assertConfigCommand(t, "target = cloud\n", homeDir, "config", "get", "target") assertConfigCommand(t, "", homeDir, "config", "set", "target", "http://127.0.0.1:8080") @@ -66,15 +69,15 @@ func TestUseAPIKey(t *testing.T) { homeDir := t.TempDir() c := Config{Home: homeDir} - assert.False(t, c.UseAPIKey("t1")) + assert.False(t, c.UseAPIKey(vespa.PublicSystem, "t1")) c.Set(apiKeyFileFlag, "/tmp/foo") - assert.True(t, c.UseAPIKey("t1")) + assert.True(t, c.UseAPIKey(vespa.PublicSystem, "t1")) c.Set(apiKeyFileFlag, "") withEnv("VESPA_CLI_API_KEY", "...", func() { require.Nil(t, c.load()) - assert.True(t, c.UseAPIKey("t1")) + assert.True(t, c.UseAPIKey(vespa.PublicSystem, "t1")) }) // Test deprecated functionality @@ -97,8 +100,8 @@ func TestUseAPIKey(t *testing.T) { withEnv("VESPA_CLI_CLOUD_SYSTEM", "public", func() { _, err := os.Create(filepath.Join(homeDir, "t2.api-key.pem")) require.Nil(t, err) - assert.True(t, c.UseAPIKey("t2")) + assert.True(t, c.UseAPIKey(vespa.PublicSystem, "t2")) require.Nil(t, ioutil.WriteFile(filepath.Join(homeDir, "auth.json"), []byte(authContent), 0600)) - assert.False(t, c.UseAPIKey("t2")) + assert.False(t, c.UseAPIKey(vespa.PublicSystem, "t2")) }) } diff --git a/client/go/cmd/curl.go b/client/go/cmd/curl.go index b66780780ed..1ede2cccae3 100644 --- a/client/go/cmd/curl.go +++ b/client/go/cmd/curl.go @@ -10,6 +10,7 @@ import ( "github.com/spf13/cobra" "github.com/vespa-engine/vespa/client/go/auth0" "github.com/vespa-engine/vespa/client/go/curl" + "github.com/vespa-engine/vespa/client/go/vespa" ) var curlDryRun bool @@ -61,8 +62,8 @@ $ vespa curl -- -v --data-urlencode "yql=select * from music where album contain if err != nil { return err } - if t.Type() == "cloud" { - if err := addCloudAuth0Authentication(cfg, c); err != nil { + if t.Type() == vespa.TargetCloud { + if err := addCloudAuth0Authentication(t.Deployment().System, cfg, c); err != nil { return err } } @@ -92,17 +93,17 @@ $ vespa curl -- -v --data-urlencode "yql=select * from music where album contain }, } -func addCloudAuth0Authentication(cfg *Config, c *curl.Command) error { - a, err := auth0.GetAuth0(cfg.AuthConfigPath(), getSystemName(), getApiURL()) +func addCloudAuth0Authentication(system vespa.System, cfg *Config, c *curl.Command) error { + a, err := auth0.GetAuth0(cfg.AuthConfigPath(), system.Name, system.URL) if err != nil { return err } - system, err := a.PrepareSystem(auth0.ContextWithCancel()) + authSystem, err := a.PrepareSystem(auth0.ContextWithCancel()) if err != nil { return err } - c.Header("Authorization", "Bearer "+system.AccessToken) + c.Header("Authorization", "Bearer "+authSystem.AccessToken) return nil } diff --git a/client/go/cmd/curl_test.go b/client/go/cmd/curl_test.go index 5709096fe37..253943f2b04 100644 --- a/client/go/cmd/curl_test.go +++ b/client/go/cmd/curl_test.go @@ -7,11 +7,12 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/vespa-engine/vespa/client/go/mock" ) func TestCurl(t *testing.T) { homeDir := filepath.Join(t.TempDir(), ".vespa") - httpClient := &mockHttpClient{} + httpClient := &mock.HTTPClient{} out, _ := execute(command{homeDir: homeDir, args: []string{"curl", "-n", "-a", "t1.a1.i1", "--", "-v", "--data-urlencode", "arg=with space", "/search"}}, t, httpClient) expected := fmt.Sprintf("curl --key %s --cert %s -v --data-urlencode 'arg=with space' http://127.0.0.1:8080/search\n", diff --git a/client/go/cmd/deploy.go b/client/go/cmd/deploy.go index 14b6e969df7..13f37fa3901 100644 --- a/client/go/cmd/deploy.go +++ b/client/go/cmd/deploy.go @@ -26,7 +26,7 @@ func init() { rootCmd.AddCommand(deployCmd) rootCmd.AddCommand(prepareCmd) rootCmd.AddCommand(activateCmd) - deployCmd.PersistentFlags().StringVarP(&zoneArg, zoneFlag, "z", "dev.aws-us-east-1c", "The zone to use for deployment") + deployCmd.PersistentFlags().StringVarP(&zoneArg, zoneFlag, "z", "", "The zone to use for deployment. This defaults to a dev zone") deployCmd.PersistentFlags().StringVarP(&logLevelArg, logLevelFlag, "l", "error", `Log level for Vespa logs. Must be "error", "warning", "info" or "debug"`) } @@ -64,7 +64,7 @@ $ vespa deploy -t cloud -z perf.aws-us-east-1c`, if err != nil { return err } - opts, err := getDeploymentOpts(cfg, pkg, target) + opts, err := getDeploymentOptions(cfg, pkg, target) if err != nil { return err } @@ -73,7 +73,7 @@ $ vespa deploy -t cloud -z perf.aws-us-east-1c`, return err } - fmt.Print("\n") + log.Println() if opts.IsCloud() { printSuccess("Triggered deployment of ", color.Cyan(pkg.Path), " with run ID ", color.Cyan(sessionOrRunID)) } else { @@ -82,9 +82,9 @@ $ vespa deploy -t cloud -z perf.aws-us-east-1c`, if opts.IsCloud() { log.Printf("\nUse %s for deployment status, or follow this deployment at", color.Cyan("vespa status")) log.Print(color.Cyan(fmt.Sprintf("%s/tenant/%s/application/%s/dev/instance/%s/job/%s-%s/run/%d", - getConsoleURL(), - opts.Deployment.Application.Tenant, opts.Deployment.Application.Application, opts.Deployment.Application.Instance, - opts.Deployment.Zone.Environment, opts.Deployment.Zone.Region, + opts.Target.Deployment().System.ConsoleURL, + opts.Target.Deployment().Application.Tenant, opts.Target.Deployment().Application.Application, opts.Target.Deployment().Application.Instance, + opts.Target.Deployment().Zone.Environment, opts.Target.Deployment().Zone.Region, sessionOrRunID))) } return waitForQueryService(sessionOrRunID) @@ -110,7 +110,7 @@ var prepareCmd = &cobra.Command{ if err != nil { return err } - sessionID, err := vespa.Prepare(vespa.DeploymentOpts{ + sessionID, err := vespa.Prepare(vespa.DeploymentOptions{ ApplicationPackage: pkg, Target: target, }) @@ -148,7 +148,7 @@ var activateCmd = &cobra.Command{ if err != nil { return err } - err = vespa.Activate(sessionID, vespa.DeploymentOpts{ + err = vespa.Activate(sessionID, vespa.DeploymentOptions{ ApplicationPackage: pkg, Target: target, }) diff --git a/client/go/cmd/deploy_test.go b/client/go/cmd/deploy_test.go index a37a433397f..f5af3751eb8 100644 --- a/client/go/cmd/deploy_test.go +++ b/client/go/cmd/deploy_test.go @@ -10,6 +10,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/vespa-engine/vespa/client/go/mock" "github.com/vespa-engine/vespa/client/go/vespa" ) @@ -32,9 +33,9 @@ func TestDeployZipWithURLTargetArgument(t *testing.T) { applicationPackage := "testdata/applications/withTarget/target/application.zip" arguments := []string{"deploy", "testdata/applications/withTarget/target/application.zip", "-t", "http://target:19071"} - client := &mockHttpClient{} + client := &mock.HTTPClient{} assert.Equal(t, - "Success: Deployed "+applicationPackage+"\n", + "\nSuccess: Deployed "+applicationPackage+"\n", executeCommand(t, client, arguments, []string{})) assertDeployRequestMade("http://target:19071", client, t) } @@ -60,7 +61,7 @@ func TestDeployApplicationDirectoryWithPomAndTarget(t *testing.T) { } func TestDeployApplicationDirectoryWithPomAndEmptyTarget(t *testing.T) { - client := &mockHttpClient{} + client := &mock.HTTPClient{} _, outErr := execute(command{args: []string{"deploy", "testdata/applications/withEmptyTarget"}}, t, client) assert.Equal(t, "Error: pom.xml exists but no target/application.zip. Run mvn package first\n", @@ -104,15 +105,15 @@ func TestDeployError(t *testing.T) { } func assertDeploy(applicationPackage string, arguments []string, t *testing.T) { - client := &mockHttpClient{} + client := &mock.HTTPClient{} assert.Equal(t, - "Success: Deployed "+applicationPackage+"\n", + "\nSuccess: Deployed "+applicationPackage+"\n", executeCommand(t, client, arguments, []string{})) assertDeployRequestMade("http://127.0.0.1:19071", client, t) } func assertPrepare(applicationPackage string, arguments []string, t *testing.T) { - client := &mockHttpClient{} + client := &mock.HTTPClient{} client.NextResponse(200, `{"session-id":"42"}`) assert.Equal(t, "Success: Prepared "+applicationPackage+" with session 42\n", @@ -120,12 +121,12 @@ func assertPrepare(applicationPackage string, arguments []string, t *testing.T) assertPackageUpload(0, "http://127.0.0.1:19071/application/v2/tenant/default/session", client, t) sessionURL := "http://127.0.0.1:19071/application/v2/tenant/default/session/42/prepared" - assert.Equal(t, sessionURL, client.requests[1].URL.String()) - assert.Equal(t, "PUT", client.requests[1].Method) + assert.Equal(t, sessionURL, client.Requests[1].URL.String()) + assert.Equal(t, "PUT", client.Requests[1].Method) } func assertActivate(applicationPackage string, arguments []string, t *testing.T) { - client := &mockHttpClient{} + client := &mock.HTTPClient{} homeDir := t.TempDir() cfg := Config{Home: filepath.Join(homeDir, ".vespa"), createDirs: true} if err := cfg.WriteSessionID(vespa.DefaultApplication, 42); err != nil { @@ -136,14 +137,14 @@ func assertActivate(applicationPackage string, arguments []string, t *testing.T) "Success: Activated "+applicationPackage+" with session 42\n", out) url := "http://127.0.0.1:19071/application/v2/tenant/default/session/42/active" - assert.Equal(t, url, client.lastRequest.URL.String()) - assert.Equal(t, "PUT", client.lastRequest.Method) + assert.Equal(t, url, client.LastRequest.URL.String()) + assert.Equal(t, "PUT", client.LastRequest.Method) } -func assertPackageUpload(requestNumber int, url string, client *mockHttpClient, t *testing.T) { - req := client.lastRequest +func assertPackageUpload(requestNumber int, url string, client *mock.HTTPClient, t *testing.T) { + req := client.LastRequest if requestNumber >= 0 { - req = client.requests[requestNumber] + req = client.Requests[requestNumber] } assert.Equal(t, url, req.URL.String()) assert.Equal(t, "application/zip", req.Header.Get("Content-Type")) @@ -155,12 +156,12 @@ func assertPackageUpload(requestNumber int, url string, client *mockHttpClient, assert.Equal(t, "PK\x03\x04\x14\x00\b", string(buf)) } -func assertDeployRequestMade(target string, client *mockHttpClient, t *testing.T) { +func assertDeployRequestMade(target string, client *mock.HTTPClient, t *testing.T) { assertPackageUpload(-1, target+"/application/v2/tenant/default/prepareandactivate", client, t) } func assertApplicationPackageError(t *testing.T, cmd string, status int, expectedMessage string, returnBody string) { - client := &mockHttpClient{} + client := &mock.HTTPClient{} client.NextResponse(status, returnBody) _, outErr := execute(command{args: []string{cmd, "testdata/applications/withTarget/target/application.zip"}}, t, client) assert.Equal(t, @@ -169,10 +170,10 @@ func assertApplicationPackageError(t *testing.T, cmd string, status int, expecte } func assertDeployServerError(t *testing.T, status int, errorMessage string) { - client := &mockHttpClient{} + client := &mock.HTTPClient{} client.NextResponse(status, errorMessage) _, outErr := execute(command{args: []string{"deploy", "testdata/applications/withTarget/target/application.zip"}}, t, client) assert.Equal(t, - "Error: error from deploy service at 127.0.0.1:19071 (Status "+strconv.Itoa(status)+"):\n"+errorMessage+"\n", + "Error: error from deploy api at 127.0.0.1:19071 (Status "+strconv.Itoa(status)+"):\n"+errorMessage+"\n", outErr) } diff --git a/client/go/cmd/document_test.go b/client/go/cmd/document_test.go index 2b596e9893b..1d650f77d08 100644 --- a/client/go/cmd/document_test.go +++ b/client/go/cmd/document_test.go @@ -10,6 +10,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/vespa-engine/vespa/client/go/mock" "github.com/vespa-engine/vespa/client/go/util" "github.com/vespa-engine/vespa/client/go/vespa" ) @@ -66,7 +67,7 @@ func TestDocumentRemoveWithoutIdArg(t *testing.T) { func TestDocumentSendMissingId(t *testing.T) { arguments := []string{"document", "put", "testdata/A-Head-Full-of-Dreams-Without-Operation.json"} - client := &mockHttpClient{} + client := &mock.HTTPClient{} _, outErr := execute(command{args: arguments}, t, client) assert.Equal(t, "Error: No document id given neither as argument or as a 'put' key in the json file\n", @@ -75,7 +76,7 @@ func TestDocumentSendMissingId(t *testing.T) { func TestDocumentSendWithDisagreeingOperations(t *testing.T) { arguments := []string{"document", "update", "testdata/A-Head-Full-of-Dreams-Put.json"} - client := &mockHttpClient{} + client := &mock.HTTPClient{} _, outErr := execute(command{args: arguments}, t, client) assert.Equal(t, "Error: Wanted document operation is update but the JSON file specifies put\n", @@ -96,7 +97,7 @@ func TestDocumentGet(t *testing.T) { } func assertDocumentSend(arguments []string, expectedOperation string, expectedMethod string, expectedDocumentId string, expectedPayloadFile string, t *testing.T) { - client := &mockHttpClient{} + client := &mock.HTTPClient{} documentURL, err := documentServiceURL(client) if err != nil { t.Fatal(err) @@ -116,16 +117,16 @@ func assertDocumentSend(arguments []string, expectedOperation string, expectedMe assert.Equal(t, expectedCurl, errOut) } assert.Equal(t, "Success: "+expectedOperation+" "+expectedDocumentId+"\n", out) - assert.Equal(t, expectedURL, client.lastRequest.URL.String()) - assert.Equal(t, "application/json", client.lastRequest.Header.Get("Content-Type")) - assert.Equal(t, expectedMethod, client.lastRequest.Method) + assert.Equal(t, expectedURL, client.LastRequest.URL.String()) + assert.Equal(t, "application/json", client.LastRequest.Header.Get("Content-Type")) + assert.Equal(t, expectedMethod, client.LastRequest.Method) expectedPayload, _ := ioutil.ReadFile(expectedPayloadFile) - assert.Equal(t, string(expectedPayload), util.ReaderToString(client.lastRequest.Body)) + assert.Equal(t, string(expectedPayload), util.ReaderToString(client.LastRequest.Body)) } func assertDocumentGet(arguments []string, documentId string, t *testing.T) { - client := &mockHttpClient{} + client := &mock.HTTPClient{} documentURL, err := documentServiceURL(client) if err != nil { t.Fatal(err) @@ -140,12 +141,12 @@ func assertDocumentGet(arguments []string, documentId string, t *testing.T) { `, executeCommand(t, client, arguments, []string{})) expectedPath, _ := vespa.IdToURLPath(documentId) - assert.Equal(t, documentURL+"/document/v1/"+expectedPath, client.lastRequest.URL.String()) - assert.Equal(t, "GET", client.lastRequest.Method) + assert.Equal(t, documentURL+"/document/v1/"+expectedPath, client.LastRequest.URL.String()) + assert.Equal(t, "GET", client.LastRequest.Method) } func assertDocumentError(t *testing.T, status int, errorMessage string) { - client := &mockHttpClient{} + client := &mock.HTTPClient{} client.NextResponse(status, errorMessage) _, outErr := execute(command{args: []string{"document", "put", "id:mynamespace:music::a-head-full-of-dreams", @@ -156,7 +157,7 @@ func assertDocumentError(t *testing.T, status int, errorMessage string) { } func assertDocumentServerError(t *testing.T, status int, errorMessage string) { - client := &mockHttpClient{} + client := &mock.HTTPClient{} client.NextResponse(status, errorMessage) _, outErr := execute(command{args: []string{"document", "put", "id:mynamespace:music::a-head-full-of-dreams", @@ -166,7 +167,7 @@ func assertDocumentServerError(t *testing.T, status int, errorMessage string) { outErr) } -func documentServiceURL(client *mockHttpClient) (string, error) { +func documentServiceURL(client *mock.HTTPClient) (string, error) { service, err := getService("document", 0, "") if err != nil { return "", err diff --git a/client/go/cmd/helpers.go b/client/go/cmd/helpers.go index 547126a8156..ab47a0e6d88 100644 --- a/client/go/cmd/helpers.go +++ b/client/go/cmd/helpers.go @@ -5,6 +5,8 @@ package cmd import ( + "crypto/tls" + "crypto/x509" "encoding/json" "fmt" "log" @@ -29,6 +31,40 @@ func printSuccess(msg ...interface{}) { log.Print(color.Green("Success: "), fmt.Sprint(msg...)) } +func athenzPath(filename string) (string, error) { + userHome, err := os.UserHomeDir() + if err != nil { + return "", err + } + return filepath.Join(userHome, ".athenz", filename), nil +} + +func athenzKeyPair() (tls.Certificate, error) { + certFile, err := athenzPath("cert") + if err != nil { + return tls.Certificate{}, err + } + keyFile, err := athenzPath("key") + if err != nil { + return tls.Certificate{}, err + } + kp, err := tls.LoadX509KeyPair(certFile, keyFile) + if err != nil { + return tls.Certificate{}, err + } + cert, err := x509.ParseCertificate(kp.Certificate[0]) + if err != nil { + return tls.Certificate{}, err + } + now := time.Now() + expiredAt := cert.NotAfter + if expiredAt.Before(now) { + delta := now.Sub(expiredAt).Truncate(time.Second) + return tls.Certificate{}, errHint(fmt.Errorf("certificate %s expired at %s (%s ago)", certFile, cert.NotAfter, delta), "Try renewing certificate with 'athenz-user-cert'") + } + return kp, nil +} + func vespaCliHome() (string, error) { home := os.Getenv("VESPA_CLI_HOME") if home == "" { @@ -59,16 +95,20 @@ func vespaCliCacheDir() (string, error) { return cacheDir, nil } -func deploymentFromArgs() (vespa.Deployment, error) { - zone, err := vespa.ZoneFromString(zoneArg) - if err != nil { - return vespa.Deployment{}, err +func deploymentFromArgs(system vespa.System) (vespa.Deployment, error) { + zone := system.DefaultZone + var err error + if zoneArg != "" { + zone, err = vespa.ZoneFromString(zoneArg) + if err != nil { + return vespa.Deployment{}, err + } } app, err := getApplication() if err != nil { return vespa.Deployment{}, err } - return vespa.Deployment{Application: app, Zone: zone}, nil + return vespa.Deployment{System: system, Application: app, Zone: zone}, nil } func applicationSource(args []string) string { @@ -124,28 +164,18 @@ func getService(service string, sessionOrRunID int64, cluster string) (*vespa.Se func getEndpointsOverride() string { return os.Getenv("VESPA_CLI_ENDPOINTS") } -func getSystem() string { return os.Getenv("VESPA_CLI_CLOUD_SYSTEM") } - -func getSystemName() string { - if getSystem() == "publiccd" { - return "publiccd" +func getSystem(targetType string) (vespa.System, error) { + name := os.Getenv("VESPA_CLI_CLOUD_SYSTEM") + if name != "" { + return vespa.GetSystem(name) } - return "public" -} - -func getConsoleURL() string { - if getSystem() == "publiccd" { - return "https://console-cd.vespa.oath.cloud" - } - return "https://console.vespa.oath.cloud" - -} - -func getApiURL() string { - if getSystem() == "publiccd" { - return "https://api.vespa-external-cd.aws.oath.cloud:4443" + switch targetType { + case vespa.TargetHosted: + return vespa.MainSystem, nil + case vespa.TargetCloud: + return vespa.PublicSystem, nil } - return "https://api.vespa-external.aws.oath.cloud:4443" + return vespa.System{}, fmt.Errorf("no default system found for %s target", targetType) } func getTarget() (vespa.Target, error) { @@ -172,53 +202,80 @@ func createTarget() (vespa.Target, error) { return vespa.CustomTarget(targetType), nil } switch targetType { - case "local": + case vespa.TargetLocal: return vespa.LocalTarget(), nil - case "cloud": - cfg, err := LoadConfig() - if err != nil { - return nil, err - } - deployment, err := deploymentFromArgs() - if err != nil { - return nil, err - } - endpoints, err := getEndpointsFromEnv() - if err != nil { - return nil, err - } + case vespa.TargetCloud, vespa.TargetHosted: + return createCloudTarget(targetType) + } + return nil, errHint(fmt.Errorf("invalid target: %s", targetType), "Valid targets are 'local', 'cloud', 'hosted' or an URL") +} - var apiKey []byte = nil - if cfg.UseAPIKey(deployment.Application.Tenant) { +func createCloudTarget(targetType string) (vespa.Target, error) { + cfg, err := LoadConfig() + if err != nil { + return nil, err + } + system, err := getSystem(targetType) + if err != nil { + return nil, err + } + deployment, err := deploymentFromArgs(system) + if err != nil { + return nil, err + } + endpoints, err := getEndpointsFromEnv() + if err != nil { + return nil, err + } + var ( + apiKey []byte + authConfigPath string + apiTLSOptions vespa.TLSOptions + deploymentTLSOptions vespa.TLSOptions + ) + if targetType == vespa.TargetCloud { + if cfg.UseAPIKey(system, deployment.Application.Tenant) { apiKey, err = cfg.ReadAPIKey(deployment.Application.Tenant) if err != nil { return nil, err } } + authConfigPath = cfg.AuthConfigPath() kp, err := cfg.X509KeyPair(deployment.Application) if err != nil { return nil, errHint(err, "Deployment to cloud requires a certificate. Try 'vespa auth cert'") } - - return vespa.CloudTarget( - getApiURL(), - deployment, - apiKey, - vespa.TLSOptions{ - KeyPair: kp.KeyPair, - CertificateFile: kp.CertificateFile, - PrivateKeyFile: kp.PrivateKeyFile, - }, - vespa.LogOptions{ - Writer: stdout, - Level: vespa.LogLevel(logLevelArg), - }, - cfg.AuthConfigPath(), - getSystemName(), - endpoints, - ), nil - } - return nil, errHint(fmt.Errorf("invalid target: %s", targetType), "Valid targets are 'local', 'cloud' or an URL") + deploymentTLSOptions = vespa.TLSOptions{ + KeyPair: kp.KeyPair, + CertificateFile: kp.CertificateFile, + PrivateKeyFile: kp.PrivateKeyFile, + } + } else if targetType == vespa.TargetHosted { + kp, err := athenzKeyPair() + if err != nil { + return nil, err + } + apiTLSOptions = vespa.TLSOptions{KeyPair: kp} + deploymentTLSOptions = apiTLSOptions + } else { + return nil, fmt.Errorf("invalid cloud target: %s", targetType) + } + apiOptions := vespa.APIOptions{ + System: system, + TLSOptions: apiTLSOptions, + APIKey: apiKey, + AuthConfigPath: authConfigPath, + } + deploymentOptions := vespa.CloudDeploymentOptions{ + Deployment: deployment, + TLSOptions: deploymentTLSOptions, + ClusterURLs: endpoints, + } + logOptions := vespa.LogOptions{ + Writer: stdout, + Level: vespa.LogLevel(logLevelArg), + } + return vespa.CloudTarget(apiOptions, deploymentOptions, logOptions) } func waitForService(service string, sessionOrRunID int64) error { @@ -242,25 +299,15 @@ func waitForService(service string, sessionOrRunID int64) error { return nil } -func getDeploymentOpts(cfg *Config, pkg vespa.ApplicationPackage, target vespa.Target) (vespa.DeploymentOpts, error) { - opts := vespa.DeploymentOpts{ApplicationPackage: pkg, Target: target} +func getDeploymentOptions(cfg *Config, pkg vespa.ApplicationPackage, target vespa.Target) (vespa.DeploymentOptions, error) { + opts := vespa.DeploymentOptions{ApplicationPackage: pkg, Target: target} if opts.IsCloud() { - deployment, err := deploymentFromArgs() - if err != nil { - return vespa.DeploymentOpts{}, err - } - if !opts.ApplicationPackage.HasCertificate() { + if target.Type() == vespa.TargetCloud && !opts.ApplicationPackage.HasCertificate() { hint := "Try 'vespa auth cert'" - return vespa.DeploymentOpts{}, errHint(fmt.Errorf("missing certificate in application package"), "Applications in Vespa Cloud require a certificate", hint) - } - if cfg.UseAPIKey(deployment.Application.Tenant) { - opts.APIKey, err = cfg.ReadAPIKey(deployment.Application.Tenant) - if err != nil { - return vespa.DeploymentOpts{}, err - } + return vespa.DeploymentOptions{}, errHint(fmt.Errorf("missing certificate in application package"), "Applications in Vespa Cloud require a certificate", hint) } - opts.Deployment = deployment } + opts.Timeout = time.Duration(waitSecsArg) * time.Second return opts, nil } diff --git a/client/go/cmd/log_test.go b/client/go/cmd/log_test.go index 1208be8f80c..3f6714b0d3c 100644 --- a/client/go/cmd/log_test.go +++ b/client/go/cmd/log_test.go @@ -7,20 +7,23 @@ import ( "github.com/stretchr/testify/assert" "github.com/vespa-engine/vespa/client/go/build" + "github.com/vespa-engine/vespa/client/go/mock" ) func TestLog(t *testing.T) { homeDir := filepath.Join(t.TempDir(), ".vespa") pkgDir := mockApplicationPackage(t, false) - httpClient := &mockHttpClient{} + httpClient := &mock.HTTPClient{} httpClient.NextResponse(200, `1632738690.905535 host1a.dev.aws-us-east-1c 806/53 logserver-container Container.com.yahoo.container.jdisc.ConfiguredApplication info Switching to the latest deployed set of configurations and components. Application config generation: 52532`) execute(command{homeDir: homeDir, args: []string{"config", "set", "application", "t1.a1.i1"}}, t, httpClient) execute(command{homeDir: homeDir, args: []string{"config", "set", "target", "cloud"}}, t, httpClient) execute(command{homeDir: homeDir, args: []string{"auth", "api-key"}}, t, httpClient) + execute(command{homeDir: homeDir, args: []string{"config", "set", "target", "cloud"}}, t, httpClient) execute(command{homeDir: homeDir, args: []string{"config", "set", "api-key-file", filepath.Join(homeDir, "t1.api-key.pem")}}, t, httpClient) - execute(command{homeDir: homeDir, args: []string{"cert", pkgDir}}, t, httpClient) + execute(command{homeDir: homeDir, args: []string{"auth", "cert", pkgDir}}, t, httpClient) - out, _ := execute(command{homeDir: homeDir, args: []string{"log", "--from", "2021-09-27T10:00:00Z", "--to", "2021-09-27T11:00:00Z"}}, t, httpClient) + out, outErr := execute(command{homeDir: homeDir, args: []string{"log", "--from", "2021-09-27T10:00:00Z", "--to", "2021-09-27T11:00:00Z"}}, t, httpClient) + assert.Equal(t, "", outErr) expected := "[2021-09-27 10:31:30.905535] host1a.dev.aws-us-east-1c info logserver-container Container.com.yahoo.container.jdisc.ConfiguredApplication Switching to the latest deployed set of configurations and components. Application config generation: 52532\n" assert.Equal(t, expected, out) @@ -34,13 +37,14 @@ func TestLogOldClient(t *testing.T) { build.Version = "7.0.0" homeDir := filepath.Join(t.TempDir(), ".vespa") pkgDir := mockApplicationPackage(t, false) - httpClient := &mockHttpClient{} + httpClient := &mock.HTTPClient{} httpClient.NextResponse(200, `{"minVersion": "8.0.0"}`) execute(command{homeDir: homeDir, args: []string{"config", "set", "application", "t1.a1.i1"}}, t, httpClient) execute(command{homeDir: homeDir, args: []string{"config", "set", "target", "cloud"}}, t, httpClient) execute(command{homeDir: homeDir, args: []string{"auth", "api-key"}}, t, httpClient) + execute(command{homeDir: homeDir, args: []string{"config", "set", "target", "cloud"}}, t, httpClient) execute(command{homeDir: homeDir, args: []string{"config", "set", "api-key-file", filepath.Join(homeDir, "t1.api-key.pem")}}, t, httpClient) - execute(command{homeDir: homeDir, args: []string{"cert", pkgDir}}, t, httpClient) + execute(command{homeDir: homeDir, args: []string{"auth", "cert", pkgDir}}, t, httpClient) out, errOut := execute(command{homeDir: homeDir, args: []string{"log"}}, t, httpClient) assert.Equal(t, "", out) expected := "Error: client version 7.0.0 is less than the minimum supported version: 8.0.0\nHint: This is not a fatal error, but this version may not work as expected\nHint: Try 'vespa version' to check for a new version\n" diff --git a/client/go/cmd/login.go b/client/go/cmd/login.go index 8787f1f80f5..2ac480d05f5 100644 --- a/client/go/cmd/login.go +++ b/client/go/cmd/login.go @@ -18,7 +18,15 @@ var loginCmd = &cobra.Command{ if err != nil { return err } - a, err := auth0.GetAuth0(cfg.AuthConfigPath(), getSystemName(), getApiURL()) + targetType, err := getTargetType() + if err != nil { + return err + } + system, err := getSystem(targetType) + if err != nil { + return err + } + a, err := auth0.GetAuth0(cfg.AuthConfigPath(), system.Name, system.URL) if err != nil { return err } diff --git a/client/go/cmd/logout.go b/client/go/cmd/logout.go index ddc1d36d5e1..b1f2477aba4 100644 --- a/client/go/cmd/logout.go +++ b/client/go/cmd/logout.go @@ -17,7 +17,15 @@ var logoutCmd = &cobra.Command{ if err != nil { return err } - a, err := auth0.GetAuth0(cfg.AuthConfigPath(), getSystemName(), getApiURL()) + targetType, err := getTargetType() + if err != nil { + return err + } + system, err := getSystem(targetType) + if err != nil { + return err + } + a, err := auth0.GetAuth0(cfg.AuthConfigPath(), system.Name, system.URL) if err != nil { return err } diff --git a/client/go/cmd/prod.go b/client/go/cmd/prod.go index 8c40eb969bf..10fc9f92368 100644 --- a/client/go/cmd/prod.go +++ b/client/go/cmd/prod.go @@ -73,6 +73,10 @@ https://cloud.vespa.ai/en/reference/deployment`, if err != nil { return fmt.Errorf("a services.xml declaring your cluster(s) must exist: %w", err) } + target, err := getTarget() + if err != nil { + return err + } fmt.Fprint(stdout, "This will modify any existing ", color.Yellow("deployment.xml"), " and ", color.Yellow("services.xml"), "!\nBefore modification a backup of the original file will be created.\n\n") @@ -80,7 +84,7 @@ https://cloud.vespa.ai/en/reference/deployment`, fmt.Fprint(stdout, "Abort the configuration at any time by pressing Ctrl-C. The\nfiles will remain untouched.\n\n") fmt.Fprint(stdout, "See this guide for sizing a Vespa deployment:\n", color.Green("https://docs.vespa.ai/en/performance/sizing-search.html\n\n")) r := bufio.NewReader(stdin) - deploymentXML, err = updateRegions(r, deploymentXML) + deploymentXML, err = updateRegions(r, deploymentXML, target.Deployment().System) if err != nil { return err } @@ -127,8 +131,9 @@ $ vespa prod submit`, if err != nil { return err } - if target.Type() != "cloud" { - return fmt.Errorf("%s target cannot deploy to Vespa Cloud", target.Type()) + if target.Type() != vespa.TargetCloud { + // TODO: Add support for hosted + return fmt.Errorf("prod submit does not support %s target", target.Type()) } appSource := applicationSource(args) pkg, err := vespa.FindApplicationPackage(appSource, true) @@ -156,7 +161,7 @@ $ vespa prod submit`, fmt.Fprintln(stderr, color.Yellow("Warning:"), "We recommend doing this only from a CD job") printErrHint(nil, "See https://cloud.vespa.ai/en/getting-to-production") } - opts, err := getDeploymentOpts(cfg, pkg, target) + opts, err := getDeploymentOptions(cfg, pkg, target) if err != nil { return err } @@ -165,7 +170,7 @@ $ vespa prod submit`, } else { printSuccess("Submitted ", color.Cyan(pkg.Path), " for deployment") log.Printf("See %s for deployment progress\n", color.Cyan(fmt.Sprintf("%s/tenant/%s/application/%s/prod/deployment", - getConsoleURL(), opts.Deployment.Application.Tenant, opts.Deployment.Application.Application))) + opts.Target.Deployment().System.ConsoleURL, opts.Target.Deployment().Application.Tenant, opts.Target.Deployment().Application.Application))) } return nil }, @@ -202,8 +207,8 @@ func writeWithBackup(pkg vespa.ApplicationPackage, filename, contents string) er return ioutil.WriteFile(dst, []byte(contents), 0644) } -func updateRegions(r *bufio.Reader, deploymentXML xml.Deployment) (xml.Deployment, error) { - regions, err := promptRegions(r, deploymentXML) +func updateRegions(r *bufio.Reader, deploymentXML xml.Deployment, system vespa.System) (xml.Deployment, error) { + regions, err := promptRegions(r, deploymentXML, system) if err != nil { return xml.Deployment{}, err } @@ -222,7 +227,7 @@ func updateRegions(r *bufio.Reader, deploymentXML xml.Deployment) (xml.Deploymen return deploymentXML, nil } -func promptRegions(r *bufio.Reader, deploymentXML xml.Deployment) (string, error) { +func promptRegions(r *bufio.Reader, deploymentXML xml.Deployment, system vespa.System) (string, error) { fmt.Fprintln(stdout, color.Cyan("> Deployment regions")) fmt.Fprintf(stdout, "Documentation: %s\n", color.Green("https://cloud.vespa.ai/en/reference/zones")) fmt.Fprintf(stdout, "Example: %s\n\n", color.Yellow("aws-us-east-1c,aws-us-west-2a")) @@ -238,7 +243,7 @@ func promptRegions(r *bufio.Reader, deploymentXML xml.Deployment) (string, error validator := func(input string) error { regions := strings.Split(input, ",") for _, r := range regions { - if !xml.IsProdRegion(r, getSystem()) { + if !xml.IsProdRegion(r, system) { return fmt.Errorf("invalid region %s", r) } } diff --git a/client/go/cmd/prod_test.go b/client/go/cmd/prod_test.go index 8fa9ef401b5..90b67af8669 100644 --- a/client/go/cmd/prod_test.go +++ b/client/go/cmd/prod_test.go @@ -10,6 +10,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/vespa-engine/vespa/client/go/mock" "github.com/vespa-engine/vespa/client/go/util" ) @@ -147,12 +148,12 @@ func TestProdSubmit(t *testing.T) { pkgDir := filepath.Join(t.TempDir(), "app") createApplication(t, pkgDir, false) - httpClient := &mockHttpClient{} + httpClient := &mock.HTTPClient{} httpClient.NextResponse(200, `ok`) execute(command{homeDir: homeDir, args: []string{"config", "set", "application", "t1.a1.i1"}}, t, httpClient) execute(command{homeDir: homeDir, args: []string{"config", "set", "target", "cloud"}}, t, httpClient) - execute(command{homeDir: homeDir, args: []string{"api-key"}}, t, httpClient) - execute(command{homeDir: homeDir, args: []string{"cert", pkgDir}}, t, httpClient) + execute(command{homeDir: homeDir, args: []string{"auth", "api-key"}}, t, httpClient) + execute(command{homeDir: homeDir, args: []string{"auth", "cert", pkgDir}}, t, httpClient) // Zipping requires relative paths, so much let command run from pkgDir, then reset cwd for subsequent tests. if cwd, err := os.Getwd(); err != nil { @@ -166,8 +167,8 @@ func TestProdSubmit(t *testing.T) { if err := os.Setenv("CI", "true"); err != nil { t.Fatal(err) } - out, err := execute(command{homeDir: homeDir, args: []string{"prod", "submit", "-k", filepath.Join(homeDir, "t1.api-key.pem")}}, t, httpClient) - assert.Equal(t, "", err) + out, outErr := execute(command{homeDir: homeDir, args: []string{"prod", "submit", "-k", filepath.Join(homeDir, "t1.api-key.pem")}}, t, httpClient) + assert.Equal(t, "", outErr) assert.Contains(t, out, "Success: Submitted") assert.Contains(t, out, "See https://console.vespa.oath.cloud/tenant/t1/application/a1/prod/deployment for deployment progress") } @@ -177,12 +178,12 @@ func TestProdSubmitWithJava(t *testing.T) { pkgDir := filepath.Join(t.TempDir(), "app") createApplication(t, pkgDir, true) - httpClient := &mockHttpClient{} + httpClient := &mock.HTTPClient{} httpClient.NextResponse(200, `ok`) execute(command{homeDir: homeDir, args: []string{"config", "set", "application", "t1.a1.i1"}}, t, httpClient) execute(command{homeDir: homeDir, args: []string{"config", "set", "target", "cloud"}}, t, httpClient) - execute(command{homeDir: homeDir, args: []string{"api-key"}}, t, httpClient) - execute(command{homeDir: homeDir, args: []string{"cert", pkgDir}}, t, httpClient) + execute(command{homeDir: homeDir, args: []string{"auth", "api-key"}}, t, httpClient) + execute(command{homeDir: homeDir, args: []string{"auth", "cert", pkgDir}}, t, httpClient) // Copy an application package pre-assembled with mvn package testAppDir := filepath.Join("testdata", "applications", "withDeployment", "target") @@ -191,7 +192,8 @@ func TestProdSubmitWithJava(t *testing.T) { testZipFile := filepath.Join(testAppDir, "application-test.zip") copyFile(t, filepath.Join(pkgDir, "target", "application-test.zip"), testZipFile) - out, _ := execute(command{homeDir: homeDir, args: []string{"prod", "submit", "-k", filepath.Join(homeDir, "t1.api-key.pem"), pkgDir}}, t, httpClient) + out, outErr := execute(command{homeDir: homeDir, args: []string{"prod", "submit", "-k", filepath.Join(homeDir, "t1.api-key.pem"), pkgDir}}, t, httpClient) + assert.Equal(t, "", outErr) assert.Contains(t, out, "Success: Submitted") assert.Contains(t, out, "See https://console.vespa.oath.cloud/tenant/t1/application/a1/prod/deployment for deployment progress") } diff --git a/client/go/cmd/query_test.go b/client/go/cmd/query_test.go index aef963121aa..d57268b248e 100644 --- a/client/go/cmd/query_test.go +++ b/client/go/cmd/query_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/vespa-engine/vespa/client/go/mock" ) func TestQuery(t *testing.T) { @@ -19,7 +20,7 @@ func TestQuery(t *testing.T) { } func TestQueryVerbose(t *testing.T) { - client := &mockHttpClient{} + client := &mock.HTTPClient{} client.NextResponse(200, "{\"query\":\"result\"}") cmd := command{args: []string{"query", "-v", "select from sources * where title contains 'foo'"}} out, errOut := execute(cmd, t, client) @@ -54,7 +55,7 @@ func TestServerError(t *testing.T) { } func assertQuery(t *testing.T, expectedQuery string, query ...string) { - client := &mockHttpClient{} + client := &mock.HTTPClient{} client.NextResponse(200, "{\"query\":\"result\"}") assert.Equal(t, "{\n \"query\": \"result\"\n}\n", @@ -62,11 +63,11 @@ func assertQuery(t *testing.T, expectedQuery string, query ...string) { "query output") queryURL, err := queryServiceURL(client) require.Nil(t, err) - assert.Equal(t, queryURL+"/search/"+expectedQuery, client.lastRequest.URL.String()) + assert.Equal(t, queryURL+"/search/"+expectedQuery, client.LastRequest.URL.String()) } func assertQueryError(t *testing.T, status int, errorMessage string) { - client := &mockHttpClient{} + client := &mock.HTTPClient{} client.NextResponse(status, errorMessage) _, outErr := execute(command{args: []string{"query", "yql=select from sources * where title contains 'foo'"}}, t, client) assert.Equal(t, @@ -76,7 +77,7 @@ func assertQueryError(t *testing.T, status int, errorMessage string) { } func assertQueryServiceError(t *testing.T, status int, errorMessage string) { - client := &mockHttpClient{} + client := &mock.HTTPClient{} client.NextResponse(status, errorMessage) _, outErr := execute(command{args: []string{"query", "yql=select from sources * where title contains 'foo'"}}, t, client) assert.Equal(t, @@ -85,7 +86,7 @@ func assertQueryServiceError(t *testing.T, status int, errorMessage string) { "error output") } -func queryServiceURL(client *mockHttpClient) (string, error) { +func queryServiceURL(client *mock.HTTPClient) (string, error) { service, err := getService("query", 0, "") if err != nil { return "", err diff --git a/client/go/cmd/root.go b/client/go/cmd/root.go index f5a846536c5..cbcbb6e5d12 100644 --- a/client/go/cmd/root.go +++ b/client/go/cmd/root.go @@ -54,7 +54,6 @@ Vespa documentation: https://docs.vespa.ai`, colorArg string quietArg bool apiKeyFileArg string - apiKeyArg string stdin io.ReadWriter = os.Stdin color = aurora.NewAurora(false) diff --git a/client/go/cmd/status_test.go b/client/go/cmd/status_test.go index 631aa511459..fe7228697c7 100644 --- a/client/go/cmd/status_test.go +++ b/client/go/cmd/status_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/vespa-engine/vespa/client/go/mock" ) func TestStatusDeployCommand(t *testing.T) { @@ -43,40 +44,40 @@ func TestStatusErrorResponse(t *testing.T) { } func assertDeployStatus(target string, args []string, t *testing.T) { - client := &mockHttpClient{} + client := &mock.HTTPClient{} assert.Equal(t, "Deploy API at "+target+" is ready\n", executeCommand(t, client, []string{"status", "deploy"}, args), "vespa status config-server") - assert.Equal(t, target+"/status.html", client.lastRequest.URL.String()) + assert.Equal(t, target+"/status.html", client.LastRequest.URL.String()) } func assertQueryStatus(target string, args []string, t *testing.T) { - client := &mockHttpClient{} + client := &mock.HTTPClient{} assert.Equal(t, "Container (query API) at "+target+" is ready\n", executeCommand(t, client, []string{"status", "query"}, args), "vespa status container") - assert.Equal(t, target+"/ApplicationStatus", client.lastRequest.URL.String()) + assert.Equal(t, target+"/ApplicationStatus", client.LastRequest.URL.String()) assert.Equal(t, "Container (query API) at "+target+" is ready\n", executeCommand(t, client, []string{"status"}, args), "vespa status (the default)") - assert.Equal(t, target+"/ApplicationStatus", client.lastRequest.URL.String()) + assert.Equal(t, target+"/ApplicationStatus", client.LastRequest.URL.String()) } func assertDocumentStatus(target string, args []string, t *testing.T) { - client := &mockHttpClient{} + client := &mock.HTTPClient{} assert.Equal(t, "Container (document API) at "+target+" is ready\n", executeCommand(t, client, []string{"status", "document"}, args), "vespa status container") - assert.Equal(t, target+"/ApplicationStatus", client.lastRequest.URL.String()) + assert.Equal(t, target+"/ApplicationStatus", client.LastRequest.URL.String()) } func assertQueryStatusError(target string, args []string, t *testing.T) { - client := &mockHttpClient{} + client := &mock.HTTPClient{} client.NextStatus(500) cmd := []string{"status", "container"} cmd = append(cmd, args...) diff --git a/client/go/cmd/test.go b/client/go/cmd/test.go index 179a8043a2a..d12059a8d12 100644 --- a/client/go/cmd/test.go +++ b/client/go/cmd/test.go @@ -25,7 +25,7 @@ import ( func init() { rootCmd.AddCommand(testCmd) - testCmd.PersistentFlags().StringVarP(&zoneArg, zoneFlag, "z", "dev.aws-us-east-1c", "The zone to use for deployment") + testCmd.PersistentFlags().StringVarP(&zoneArg, zoneFlag, "z", "", "The zone to use for deployment. This defaults to a dev zone") } var testCmd = &cobra.Command{ @@ -35,7 +35,7 @@ var testCmd = &cobra.Command{ Runs all JSON test files in the specified directory, or the single JSON test file specified. -See https://cloud.vespa.ai/en/reference/testing.html for details.`, +See https://docs.vespa.ai/en/reference/testing.html for details.`, Example: `$ vespa test src/test/application/tests/system-test $ vespa test src/test/application/tests/system-test/feed-and-query.json`, Args: cobra.ExactArgs(1), @@ -71,11 +71,11 @@ func runTests(rootPath string, dryRun bool) (int, []string, error) { count := 0 failed := make([]string, 0) if stat, err := os.Stat(rootPath); err != nil { - return 0, nil, errHint(err, "See https://cloud.vespa.ai/en/reference/testing") + return 0, nil, errHint(err, "See https://docs.vespa.ai/en/reference/testing") } else if stat.IsDir() { tests, err := ioutil.ReadDir(rootPath) // TODO: Use os.ReadDir when >= 1.16 is required. if err != nil { - return 0, nil, errHint(err, "See https://cloud.vespa.ai/en/reference/testing") + return 0, nil, errHint(err, "See https://docs.vespa.ai/en/reference/testing") } context := testContext{testsPath: rootPath, dryRun: dryRun} previousFailed := false @@ -108,7 +108,7 @@ func runTests(rootPath string, dryRun bool) (int, []string, error) { count++ } if count == 0 { - return 0, nil, errHint(fmt.Errorf("failed to find any tests at %s", rootPath), "See https://cloud.vespa.ai/en/reference/testing") + return 0, nil, errHint(fmt.Errorf("failed to find any tests at %s", rootPath), "See https://docs.vespa.ai/en/reference/testing") } return count, failed, nil } @@ -118,10 +118,10 @@ func runTest(testPath string, context testContext) (string, error) { var test test testBytes, err := ioutil.ReadFile(testPath) if err != nil { - return "", errHint(err, "See https://cloud.vespa.ai/en/reference/testing") + return "", errHint(err, "See https://docs.vespa.ai/en/reference/testing") } if err = json.Unmarshal(testBytes, &test); err != nil { - return "", errHint(fmt.Errorf("failed parsing test at %s: %w", testPath, err), "See https://cloud.vespa.ai/en/reference/testing") + return "", errHint(fmt.Errorf("failed parsing test at %s: %w", testPath, err), "See https://docs.vespa.ai/en/reference/testing") } testName := test.Name @@ -135,12 +135,12 @@ func runTest(testPath string, context testContext) (string, error) { defaultParameters, err := getParameters(test.Defaults.ParametersRaw, filepath.Dir(testPath)) if err != nil { fmt.Fprintln(stderr) - return "", errHint(fmt.Errorf("invalid default parameters for %s: %w", testName, err), "See https://cloud.vespa.ai/en/reference/testing") + return "", errHint(fmt.Errorf("invalid default parameters for %s: %w", testName, err), "See https://docs.vespa.ai/en/reference/testing") } if len(test.Steps) == 0 { fmt.Fprintln(stderr) - return "", errHint(fmt.Errorf("a test must have at least one step, but none were found in %s", testPath), "See https://cloud.vespa.ai/en/reference/testing") + return "", errHint(fmt.Errorf("a test must have at least one step, but none were found in %s", testPath), "See https://docs.vespa.ai/en/reference/testing") } for i, step := range test.Steps { stepName := fmt.Sprintf("Step %d", i+1) @@ -150,7 +150,7 @@ func runTest(testPath string, context testContext) (string, error) { failure, longFailure, err := verify(step, test.Defaults.Cluster, defaultParameters, context) if err != nil { fmt.Fprintln(stderr) - return "", errHint(fmt.Errorf("error in %s: %w", stepName, err), "See https://cloud.vespa.ai/en/reference/testing") + return "", errHint(fmt.Errorf("error in %s: %w", stepName, err), "See https://docs.vespa.ai/en/reference/testing") } if !context.dryRun { if failure != "" { @@ -206,6 +206,9 @@ func verify(step step, defaultCluster string, defaultParameters map[string]strin return "", "", err } externalEndpoint := requestUrl.IsAbs() + if !externalEndpoint && filepath.Base(context.testsPath) == "production-test" { + return "", "", fmt.Errorf("production tests may not specify requests against Vespa endpoints") + } if !externalEndpoint && !context.dryRun { target, err := context.target() if err != nil { diff --git a/client/go/cmd/test_test.go b/client/go/cmd/test_test.go index 9c161c091ec..1f7d0cff7b2 100644 --- a/client/go/cmd/test_test.go +++ b/client/go/cmd/test_test.go @@ -15,12 +15,13 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/vespa-engine/vespa/client/go/mock" "github.com/vespa-engine/vespa/client/go/util" "github.com/vespa-engine/vespa/client/go/vespa" ) func TestSuite(t *testing.T) { - client := &mockHttpClient{} + client := &mock.HTTPClient{} searchResponse, _ := ioutil.ReadFile("testdata/tests/response.json") client.NextStatus(200) client.NextStatus(200) @@ -45,16 +46,25 @@ func TestSuite(t *testing.T) { } func TestIllegalFileReference(t *testing.T) { - client := &mockHttpClient{} + client := &mock.HTTPClient{} client.NextStatus(200) client.NextStatus(200) _, errBytes := execute(command{args: []string{"test", "testdata/tests/production-test/illegal-reference.json"}}, t, client) - assertRequests([]*http.Request{createRequest("GET", "http://127.0.0.1:8080/search/", "{}")}, client, t) - assert.Equal(t, "\nError: error in Step 2: path may not point outside src/test/application, but 'foo/../../../../this-is-not-ok.json' does\nHint: See https://cloud.vespa.ai/en/reference/testing\n", errBytes) + assertRequests([]*http.Request{createRequest("GET", "https://domain.tld", "{}")}, client, t) + assert.Equal(t, "\nError: error in Step 2: path may not point outside src/test/application, but 'foo/../../../../this-is-not-ok.json' does\nHint: See https://docs.vespa.ai/en/reference/testing\n", errBytes) +} + +func TestIllegalRequestUri(t *testing.T) { + client := &mock.HTTPClient{} + client.NextStatus(200) + client.NextStatus(200) + _, errBytes := execute(command{args: []string{"test", "testdata/tests/production-test/illegal-uri.json"}}, t, client) + assertRequests([]*http.Request{createRequest("GET", "https://domain.tld/my-api", "")}, client, t) + assert.Equal(t, "\nError: error in Step 2: production tests may not specify requests against Vespa endpoints\nHint: See https://docs.vespa.ai/en/reference/testing\n", errBytes) } func TestProductionTest(t *testing.T) { - client := &mockHttpClient{} + client := &mock.HTTPClient{} client.NextStatus(200) outBytes, errBytes := execute(command{args: []string{"test", "testdata/tests/production-test/external.json"}}, t, client) assert.Equal(t, "external.json: . OK\n\nSuccess: 1 test OK\n", outBytes) @@ -63,19 +73,19 @@ func TestProductionTest(t *testing.T) { } func TestTestWithoutAssertions(t *testing.T) { - client := &mockHttpClient{} + client := &mock.HTTPClient{} _, errBytes := execute(command{args: []string{"test", "testdata/tests/system-test/foo/query.json"}}, t, client) - assert.Equal(t, "\nError: a test must have at least one step, but none were found in testdata/tests/system-test/foo/query.json\nHint: See https://cloud.vespa.ai/en/reference/testing\n", errBytes) + assert.Equal(t, "\nError: a test must have at least one step, but none were found in testdata/tests/system-test/foo/query.json\nHint: See https://docs.vespa.ai/en/reference/testing\n", errBytes) } func TestSuiteWithoutTests(t *testing.T) { - client := &mockHttpClient{} + client := &mock.HTTPClient{} _, errBytes := execute(command{args: []string{"test", "testdata/tests/staging-test"}}, t, client) - assert.Equal(t, "Error: failed to find any tests at testdata/tests/staging-test\nHint: See https://cloud.vespa.ai/en/reference/testing\n", errBytes) + assert.Equal(t, "Error: failed to find any tests at testdata/tests/staging-test\nHint: See https://docs.vespa.ai/en/reference/testing\n", errBytes) } func TestSingleTest(t *testing.T) { - client := &mockHttpClient{} + client := &mock.HTTPClient{} searchResponse, _ := ioutil.ReadFile("testdata/tests/response.json") client.NextStatus(200) client.NextStatus(200) @@ -112,7 +122,7 @@ func TestSingleTestWithCloudAndEndpoints(t *testing.T) { ioutil.WriteFile(keyFile, kp.PrivateKey, 0600) ioutil.WriteFile(certFile, kp.Certificate, 0600) - client := &mockHttpClient{} + client := &mock.HTTPClient{} searchResponse, _ := ioutil.ReadFile("testdata/tests/response.json") client.NextStatus(200) client.NextStatus(200) @@ -149,10 +159,10 @@ func createRequest(method string, uri string, body string) *http.Request { } } -func assertRequests(requests []*http.Request, client *mockHttpClient, t *testing.T) { - if assert.Equal(t, len(requests), len(client.requests)) { +func assertRequests(requests []*http.Request, client *mock.HTTPClient, t *testing.T) { + if assert.Equal(t, len(requests), len(client.Requests)) { for i, e := range requests { - a := client.requests[i] + a := client.Requests[i] assert.Equal(t, e.URL.String(), a.URL.String()) assert.Equal(t, e.Method, a.Method) assert.Equal(t, util.ReaderToJSON(e.Body), util.ReaderToJSON(a.Body)) diff --git a/client/go/cmd/testdata/tests/production-test/illegal-reference.json b/client/go/cmd/testdata/tests/production-test/illegal-reference.json index edd8a2fafeb..ced4a86dd6c 100644 --- a/client/go/cmd/testdata/tests/production-test/illegal-reference.json +++ b/client/go/cmd/testdata/tests/production-test/illegal-reference.json @@ -2,11 +2,13 @@ "steps": [ { "request": { + "uri": "https://domain.tld", "body": "foo/../../../empty.json" } }, { "request": { + "uri": "https://domain.tld", "body": "foo/../../../../this-is-not-ok.json" } } diff --git a/client/go/cmd/testdata/tests/production-test/illegal-uri.json b/client/go/cmd/testdata/tests/production-test/illegal-uri.json new file mode 100644 index 00000000000..fe4fadfa93d --- /dev/null +++ b/client/go/cmd/testdata/tests/production-test/illegal-uri.json @@ -0,0 +1,14 @@ +{ + "steps": [ + { + "request": { + "uri": "https://domain.tld/my-api" + } + }, + { + "request": { + "uri": "/my-api" + } + } + ] +}
\ No newline at end of file diff --git a/client/go/cmd/version_test.go b/client/go/cmd/version_test.go index 039f75a6ecd..9c05b130e84 100644 --- a/client/go/cmd/version_test.go +++ b/client/go/cmd/version_test.go @@ -6,11 +6,12 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/vespa-engine/vespa/client/go/mock" "github.com/vespa-engine/vespa/client/go/util" ) func TestVersion(t *testing.T) { - c := &mockHttpClient{} + c := &mock.HTTPClient{} c.NextResponse(200, `[{"tag_name": "v1.2.3", "published_at": "2021-09-10T12:00:00Z"}]`) util.ActiveHttpClient = c @@ -21,7 +22,7 @@ func TestVersion(t *testing.T) { } func TestVersionCheckHomebrew(t *testing.T) { - c := &mockHttpClient{} + c := &mock.HTTPClient{} c.NextResponse(200, `[{"tag_name": "v1.2.3", "published_at": "2021-09-10T12:00:00Z"}]`) util.ActiveHttpClient = c diff --git a/client/go/mock/mock.go b/client/go/mock/mock.go new file mode 100644 index 00000000000..f2fcf9c5960 --- /dev/null +++ b/client/go/mock/mock.go @@ -0,0 +1,55 @@ +package mock + +import ( + "bytes" + "crypto/tls" + "io/ioutil" + "net/http" + "strconv" + "time" +) + +type HTTPClient struct { + // The responses to return for future requests. Once a response is consumed, it's removed from this slice + nextResponses []httpResponse + + // LastRequest is the last HTTP request made through this + LastRequest *http.Request + + // Requests contains all requests made through this + Requests []*http.Request +} + +type httpResponse struct { + status int + body []byte +} + +func (c *HTTPClient) NextStatus(status int) { c.NextResponseBytes(status, nil) } + +func (c *HTTPClient) NextResponse(status int, body string) { + c.NextResponseBytes(status, []byte(body)) +} + +func (c *HTTPClient) NextResponseBytes(status int, body []byte) { + c.nextResponses = append(c.nextResponses, httpResponse{status: status, body: body}) +} + +func (c *HTTPClient) Do(request *http.Request, timeout time.Duration) (*http.Response, error) { + response := httpResponse{status: 200} + if len(c.nextResponses) > 0 { + response = c.nextResponses[0] + c.nextResponses = c.nextResponses[1:] + } + c.LastRequest = request + c.Requests = append(c.Requests, request) + return &http.Response{ + Status: "Status " + strconv.Itoa(response.status), + StatusCode: response.status, + Body: ioutil.NopCloser(bytes.NewBuffer(response.body)), + Header: make(http.Header), + }, + nil +} + +func (c *HTTPClient) UseCertificate(certificates []tls.Certificate) {} diff --git a/client/go/vespa/deploy.go b/client/go/vespa/deploy.go index aed430399a0..3316dcac924 100644 --- a/client/go/vespa/deploy.go +++ b/client/go/vespa/deploy.go @@ -38,15 +38,15 @@ type ZoneID struct { } type Deployment struct { + System System Application ApplicationID Zone ZoneID } -type DeploymentOpts struct { - ApplicationPackage ApplicationPackage +type DeploymentOptions struct { Target Target - Deployment Deployment - APIKey []byte + ApplicationPackage ApplicationPackage + Timeout time.Duration } type ApplicationPackage struct { @@ -66,13 +66,16 @@ func (d Deployment) String() string { return fmt.Sprintf("deployment of %s in %s", d.Application, d.Zone) } -func (d DeploymentOpts) String() string { - return fmt.Sprintf("%s to %s", d.Deployment, d.Target.Type()) +func (d DeploymentOptions) String() string { + return fmt.Sprintf("%s to %s", d.Target.Deployment(), d.Target.Type()) } -func (d *DeploymentOpts) IsCloud() bool { return d.Target.Type() == cloudTargetType } +// IsCloud returns whether this is a deployment to Vespa Cloud or hosted Vespa +func (d *DeploymentOptions) IsCloud() bool { + return d.Target.Type() == TargetCloud || d.Target.Type() == TargetHosted +} -func (d *DeploymentOpts) url(path string) (*url.URL, error) { +func (d *DeploymentOptions) url(path string) (*url.URL, error) { service, err := d.Target.Service(deployService, 0, 0, "") if err != nil { return nil, err @@ -196,7 +199,7 @@ func ZoneFromString(s string) (ZoneID, error) { } // Prepare deployment and return the session ID -func Prepare(deployment DeploymentOpts) (int64, error) { +func Prepare(deployment DeploymentOptions) (int64, error) { if deployment.IsCloud() { return 0, fmt.Errorf("prepare is not supported with %s target", deployment.Target.Type()) } @@ -229,7 +232,7 @@ func Prepare(deployment DeploymentOpts) (int64, error) { } // Activate deployment with sessionID from a past prepare -func Activate(sessionID int64, deployment DeploymentOpts) error { +func Activate(sessionID int64, deployment DeploymentOptions) error { if deployment.IsCloud() { return fmt.Errorf("activate is not supported with %s target", deployment.Target.Type()) } @@ -250,21 +253,21 @@ func Activate(sessionID int64, deployment DeploymentOpts) error { return checkResponse(req, response, serviceDescription) } -func Deploy(opts DeploymentOpts) (int64, error) { +func Deploy(opts DeploymentOptions) (int64, error) { path := "/application/v2/tenant/default/prepareandactivate" if opts.IsCloud() { if err := checkDeploymentOpts(opts); err != nil { return 0, err } - if opts.Deployment.Zone.Environment == "" || opts.Deployment.Zone.Region == "" { + if opts.Target.Deployment().Zone.Environment == "" || opts.Target.Deployment().Zone.Region == "" { return 0, fmt.Errorf("%s: missing zone", opts) } path = fmt.Sprintf("/application/v4/tenant/%s/application/%s/instance/%s/deploy/%s-%s", - opts.Deployment.Application.Tenant, - opts.Deployment.Application.Application, - opts.Deployment.Application.Instance, - opts.Deployment.Zone.Environment, - opts.Deployment.Zone.Region) + opts.Target.Deployment().Application.Tenant, + opts.Target.Deployment().Application.Application, + opts.Target.Deployment().Application.Instance, + opts.Target.Deployment().Zone.Environment, + opts.Target.Deployment().Zone.Region) } u, err := opts.url(path) if err != nil { @@ -290,14 +293,14 @@ func copyToPart(dst *multipart.Writer, src io.Reader, fieldname, filename string return nil } -func Submit(opts DeploymentOpts) error { +func Submit(opts DeploymentOptions) error { if !opts.IsCloud() { - return fmt.Errorf("%s: submit is unsupported", opts) + return fmt.Errorf("%s: submit is unsupported by %s target", opts, opts.Target.Type()) } if err := checkDeploymentOpts(opts); err != nil { return err } - path := fmt.Sprintf("/application/v4/tenant/%s/application/%s/submit", opts.Deployment.Application.Tenant, opts.Deployment.Application.Application) + path := fmt.Sprintf("/application/v4/tenant/%s/application/%s/submit", opts.Target.Deployment().Application.Tenant, opts.Target.Deployment().Application.Application) u, err := opts.url(path) if err != nil { return err @@ -332,7 +335,7 @@ func Submit(opts DeploymentOpts) error { } request.Header.Set("Content-Type", writer.FormDataContentType()) serviceDescription := "Submit service" - sigKeyId := opts.Deployment.Application.SerializedForm() + sigKeyId := opts.Target.Deployment().Application.SerializedForm() if err := opts.Target.SignRequest(request, sigKeyId); err != nil { return err } @@ -344,14 +347,14 @@ func Submit(opts DeploymentOpts) error { return checkResponse(request, response, serviceDescription) } -func checkDeploymentOpts(opts DeploymentOpts) error { - if !opts.ApplicationPackage.HasCertificate() { +func checkDeploymentOpts(opts DeploymentOptions) error { + if opts.Target.Type() == TargetCloud && !opts.ApplicationPackage.HasCertificate() { return fmt.Errorf("%s: missing certificate in package", opts) } return nil } -func uploadApplicationPackage(url *url.URL, opts DeploymentOpts) (int64, error) { +func uploadApplicationPackage(url *url.URL, opts DeploymentOptions) (int64, error) { zipReader, err := opts.ApplicationPackage.zipReader(false) if err != nil { return 0, err @@ -364,15 +367,18 @@ func uploadApplicationPackage(url *url.URL, opts DeploymentOpts) (int64, error) Header: header, Body: ioutil.NopCloser(zipReader), } - serviceDescription := "Deploy service" - sigKeyId := opts.Deployment.Application.SerializedForm() - if err := opts.Target.SignRequest(request, sigKeyId); err != nil { + service, err := opts.Target.Service(deployService, opts.Timeout, 0, "") + if err != nil { return 0, err } + keyID := opts.Target.Deployment().Application.SerializedForm() + if err := opts.Target.SignRequest(request, keyID); err != nil { + return 0, err + } var response *http.Response err = util.Spinner("Uploading application package ...", func() error { - response, err = util.HttpDo(request, time.Minute*10, serviceDescription) + response, err = service.Do(request, time.Minute*10) return err }) if err != nil { @@ -385,7 +391,7 @@ func uploadApplicationPackage(url *url.URL, opts DeploymentOpts) (int64, error) RunID int64 `json:"run"` // Controller } jsonResponse.SessionID = "0" // Set a default session ID for responses that don't contain int (e.g. cloud deployment) - if err := checkResponse(request, response, serviceDescription); err != nil { + if err := checkResponse(request, response, service.Description()); err != nil { return 0, err } jsonDec := json.NewDecoder(response.Body) diff --git a/client/go/vespa/system.go b/client/go/vespa/system.go new file mode 100644 index 00000000000..a6cf1a9d9ff --- /dev/null +++ b/client/go/vespa/system.go @@ -0,0 +1,68 @@ +package vespa + +import "fmt" + +// PublicSystem represents the main Vespa Cloud system. +var PublicSystem = System{ + Name: "public", + URL: "https://api.vespa-external.aws.oath.cloud:4443", + ConsoleURL: "https://console.vespa.oath.cloud", + DefaultZone: ZoneID{Environment: "dev", Region: "aws-us-east-1c"}, +} + +// PublicCDSystem represents the CD variant of the Vespa Cloud system. +var PublicCDSystem = System{ + Name: "publiccd", + URL: "https://api.vespa-external-cd.aws.oath.cloud:4443", + ConsoleURL: "https://console-cd.vespa.oath.cloud", + DefaultZone: ZoneID{Environment: "dev", Region: "aws-us-east-1c"}, +} + +// MainSystem represents the main hosted Vespa system. +var MainSystem = System{ + Name: "main", + URL: "https://api.vespa.ouryahoo.com:4443", + ConsoleURL: "https://console.vespa.ouryahoo.com", + DefaultZone: ZoneID{Environment: "dev", Region: "us-east-1"}, + AthenzDomain: "vespa.vespa", +} + +// CDSystem represents the CD variant of the hosted Vespa system. +var CDSystem = System{ + Name: "cd", + URL: "https://api-cd.vespa.ouryahoo.com:4443", + ConsoleURL: "https://console-cd.vespa.ouryahoo.com", + DefaultZone: ZoneID{Environment: "dev", Region: "cd-us-west-1"}, + AthenzDomain: "vespa.vespa.cd", +} + +// System represents a Vespa system. +type System struct { + Name string + // URL is the API URL for this system. + URL string + ConsoleURL string + // DefaultZone is default zone to use in manual deployments to this system. + DefaultZone ZoneID + // AthenzDomain is the Athenz domain used by this system. This is empty for systems not using Athenz for tenant + // authentication. + AthenzDomain string +} + +// IsPublic returns whether system s is a public (Vespa Cloud) system. +func (s *System) IsPublic() bool { return s.Name == PublicSystem.Name || s.Name == PublicCDSystem.Name } + +// GetSystem returns the system of given name. +func GetSystem(name string) (System, error) { + switch name { + case "cd": + return CDSystem, nil + case "main": + return MainSystem, nil + case "public": + return PublicSystem, nil + case "publiccd": + return PublicCDSystem, nil + } + return System{}, fmt.Errorf("invalid system: %s", name) +} diff --git a/client/go/vespa/target.go b/client/go/vespa/target.go index 204dbc143c6..f620f3b865c 100644 --- a/client/go/vespa/target.go +++ b/client/go/vespa/target.go @@ -19,12 +19,21 @@ import ( "github.com/vespa-engine/vespa/client/go/auth0" "github.com/vespa-engine/vespa/client/go/util" "github.com/vespa-engine/vespa/client/go/version" + "github.com/vespa-engine/vespa/client/go/zts" ) const ( - localTargetType = "local" - customTargetType = "custom" - cloudTargetType = "cloud" + // A target for a local Vespa service + TargetLocal = "local" + + // A target for a custom URL + TargetCustom = "custom" + + // A Vespa Cloud target + TargetCloud = "cloud" + + // A hosted Vespa target + TargetHosted = "hosted" deployService = "deploy" queryService = "query" @@ -33,16 +42,12 @@ const ( retryInterval = 2 * time.Second ) -const ( - CloudAuthApiKey = "api-key" - CloudAuthAccessToken = "access-token" -) - // Service represents a Vespa service. type Service struct { BaseURL string Name string TLSOptions TLSOptions + ztsClient ztsClient } // Target represents a Vespa platform, running named Vespa services. @@ -50,6 +55,9 @@ type Target interface { // Type returns this target's type, e.g. local or cloud. Type() string + // Deployment returns the deployment managed by this target. + Deployment() Deployment + // Service returns the service for given name. If timeout is non-zero, wait for the service to converge. Service(name string, timeout time.Duration, sessionOrRunID int64, cluster string) (*Service, error) @@ -63,11 +71,12 @@ type Target interface { CheckVersion(clientVersion version.Version) error } -// TLSOptions configures the certificate to use for service requests. +// TLSOptions configures the client certificate to use for cloud API or service requests. type TLSOptions struct { KeyPair tls.Certificate CertificateFile string PrivateKeyFile string + AthenzDomain string } // LogOptions configures the log output to produce when writing log messages. @@ -80,20 +89,41 @@ type LogOptions struct { Level int } +// CloudOptions configures URL and authentication for a cloud target. +type APIOptions struct { + System System + TLSOptions TLSOptions + APIKey []byte + AuthConfigPath string +} + +// CloudDeploymentOptions configures the deployment to manage through a cloud target. +type CloudDeploymentOptions struct { + Deployment Deployment + TLSOptions TLSOptions + ClusterURLs map[string]string // Endpoints keyed on cluster name +} + type customTarget struct { targetType string baseURL string } -func (t *customTarget) SignRequest(req *http.Request, sigKeyId string) error { return nil } - -func (t *customTarget) CheckVersion(version version.Version) error { return nil } - // Do sends request to this service. Any required authentication happens automatically. func (s *Service) Do(request *http.Request, timeout time.Duration) (*http.Response, error) { if s.TLSOptions.KeyPair.Certificate != nil { util.ActiveHttpClient.UseCertificate([]tls.Certificate{s.TLSOptions.KeyPair}) } + if s.TLSOptions.AthenzDomain != "" { + accessToken, err := s.ztsClient.AccessToken(s.TLSOptions.AthenzDomain, s.TLSOptions.KeyPair) + if err != nil { + return nil, err + } + if request.Header == nil { + request.Header = make(http.Header) + } + request.Header.Add("Authorization", "Bearer "+accessToken) + } return util.HttpDo(request, timeout, s.Description()) } @@ -130,6 +160,8 @@ func (s *Service) Description() string { func (t *customTarget) Type() string { return t.targetType } +func (t *customTarget) Deployment() Deployment { return Deployment{} } + func (t *customTarget) Service(name string, timeout time.Duration, sessionOrRunID int64, cluster string) (*Service, error) { if timeout > 0 && name != deployService { if err := t.waitForConvergence(timeout); err != nil { @@ -148,9 +180,13 @@ func (t *customTarget) Service(name string, timeout time.Duration, sessionOrRunI } func (t *customTarget) PrintLog(options LogOptions) error { - return fmt.Errorf("reading logs from non-cloud deployment is currently unsupported") + return fmt.Errorf("reading logs from non-cloud deployment is unsupported") } +func (t *customTarget) SignRequest(req *http.Request, sigKeyId string) error { return nil } + +func (t *customTarget) CheckVersion(version version.Version) error { return nil } + func (t *customTarget) urlWithPort(serviceName string) (string, error) { u, err := url.Parse(t.baseURL) if err != nil { @@ -203,32 +239,30 @@ func (t *customTarget) waitForConvergence(timeout time.Duration) error { } type cloudTarget struct { - apiURL string - targetType string - deployment Deployment - apiKey []byte - tlsOptions TLSOptions - logOptions LogOptions + apiOptions APIOptions + deploymentOptions CloudDeploymentOptions + logOptions LogOptions + ztsClient ztsClient +} - urlsByCluster map[string]string - authConfigPath string - systemName string +type ztsClient interface { + AccessToken(domain string, certficiate tls.Certificate) (string, error) } func (t *cloudTarget) resolveEndpoint(cluster string) (string, error) { if cluster == "" { - for _, u := range t.urlsByCluster { - if len(t.urlsByCluster) == 1 { + for _, u := range t.deploymentOptions.ClusterURLs { + if len(t.deploymentOptions.ClusterURLs) == 1 { return u, nil } else { - return "", fmt.Errorf("multiple clusters, none chosen: %v", t.urlsByCluster) + return "", fmt.Errorf("multiple clusters, none chosen: %v", t.deploymentOptions.ClusterURLs) } } } else { - u := t.urlsByCluster[cluster] + u := t.deploymentOptions.ClusterURLs[cluster] if u == "" { - clusters := make([]string, len(t.urlsByCluster)) - for c := range t.urlsByCluster { + clusters := make([]string, len(t.deploymentOptions.ClusterURLs)) + for c := range t.deploymentOptions.ClusterURLs { clusters = append(clusters, c) } return "", fmt.Errorf("unknown cluster '%s': must be one of %v", cluster, clusters) @@ -239,54 +273,57 @@ func (t *cloudTarget) resolveEndpoint(cluster string) (string, error) { return "", fmt.Errorf("no endpoints") } -func (t *cloudTarget) Type() string { return t.targetType } +func (t *cloudTarget) Type() string { + switch t.apiOptions.System.Name { + case MainSystem.Name, CDSystem.Name: + return TargetHosted + } + return TargetCloud +} + +func (t *cloudTarget) Deployment() Deployment { return t.deploymentOptions.Deployment } func (t *cloudTarget) Service(name string, timeout time.Duration, runID int64, cluster string) (*Service, error) { - if name != deployService && t.urlsByCluster == nil { + if name != deployService && t.deploymentOptions.ClusterURLs == nil { if err := t.waitForEndpoints(timeout, runID); err != nil { return nil, err } } switch name { case deployService: - return &Service{Name: name, BaseURL: t.apiURL}, nil - case queryService: - queryURL, err := t.resolveEndpoint(cluster) - if err != nil { - return nil, err - } - return &Service{Name: name, BaseURL: queryURL, TLSOptions: t.tlsOptions}, nil - case documentService: - documentURL, err := t.resolveEndpoint(cluster) + return &Service{Name: name, BaseURL: t.apiOptions.System.URL, TLSOptions: t.apiOptions.TLSOptions, ztsClient: t.ztsClient}, nil + case queryService, documentService: + url, err := t.resolveEndpoint(cluster) if err != nil { return nil, err } - return &Service{Name: name, BaseURL: documentURL, TLSOptions: t.tlsOptions}, nil + t.deploymentOptions.TLSOptions.AthenzDomain = t.apiOptions.System.AthenzDomain + return &Service{Name: name, BaseURL: url, TLSOptions: t.deploymentOptions.TLSOptions, ztsClient: t.ztsClient}, nil } return nil, fmt.Errorf("unknown service: %s", name) } -// SignRequest adds authentication data to a http.Request. -// The api key is used if set on cloudTarget, if not the Auth0 device flow is used. -func (t *cloudTarget) SignRequest(req *http.Request, sigKeyId string) error { - if t.apiKey != nil { - signer := NewRequestSigner(sigKeyId, t.apiKey) - if err := signer.SignRequest(req); err != nil { - return err +func (t *cloudTarget) SignRequest(req *http.Request, keyID string) error { + if t.apiOptions.System.IsPublic() { + if t.apiOptions.APIKey != nil { + signer := NewRequestSigner(keyID, t.apiOptions.APIKey) + return signer.SignRequest(req) + } else { + return t.addAuth0AccessToken(req) } } else { - if err := t.addAuth0AccessToken(req); err != nil { - return err + if t.apiOptions.TLSOptions.KeyPair.Certificate == nil { + return fmt.Errorf("system %s requires a certificate for authentication", t.apiOptions.System.Name) } + return nil } - return nil } func (t *cloudTarget) CheckVersion(clientVersion version.Version) error { if clientVersion.IsZero() { // development version is always fine return nil } - req, err := http.NewRequest("GET", fmt.Sprintf("%s/cli/v1/", t.apiURL), nil) + req, err := http.NewRequest("GET", fmt.Sprintf("%s/cli/v1/", t.apiOptions.System.URL), nil) if err != nil { return err } @@ -313,7 +350,7 @@ func (t *cloudTarget) CheckVersion(clientVersion version.Version) error { } func (t *cloudTarget) addAuth0AccessToken(request *http.Request) error { - a, err := auth0.GetAuth0(t.authConfigPath, t.systemName, t.apiURL) + a, err := auth0.GetAuth0(t.apiOptions.AuthConfigPath, t.apiOptions.System.Name, t.apiOptions.System.URL) if err != nil { return err } @@ -327,9 +364,9 @@ func (t *cloudTarget) addAuth0AccessToken(request *http.Request) error { func (t *cloudTarget) logsURL() string { return fmt.Sprintf("%s/application/v4/tenant/%s/application/%s/instance/%s/environment/%s/region/%s/logs", - t.apiURL, - t.deployment.Application.Tenant, t.deployment.Application.Application, t.deployment.Application.Instance, - t.deployment.Zone.Environment, t.deployment.Zone.Region) + t.apiOptions.System.URL, + t.deploymentOptions.Deployment.Application.Tenant, t.deploymentOptions.Deployment.Application.Application, t.deploymentOptions.Deployment.Application.Instance, + t.deploymentOptions.Deployment.Zone.Environment, t.deploymentOptions.Deployment.Zone.Region) } func (t *cloudTarget) PrintLog(options LogOptions) error { @@ -347,7 +384,7 @@ func (t *cloudTarget) PrintLog(options LogOptions) error { q.Set("to", strconv.FormatInt(toMillis, 10)) } req.URL.RawQuery = q.Encode() - t.SignRequest(req, t.deployment.Application.SerializedForm()) + t.SignRequest(req, t.deploymentOptions.Deployment.Application.SerializedForm()) return req } logFunc := func(status int, response []byte) (bool, error) { @@ -376,7 +413,7 @@ func (t *cloudTarget) PrintLog(options LogOptions) error { if options.Follow { timeout = math.MaxInt64 // No timeout } - _, err = wait(logFunc, requestFunc, &t.tlsOptions.KeyPair, timeout) + _, err = wait(logFunc, requestFunc, &t.apiOptions.TLSOptions.KeyPair, timeout) return err } @@ -391,9 +428,9 @@ func (t *cloudTarget) waitForEndpoints(timeout time.Duration, runID int64) error func (t *cloudTarget) waitForRun(runID int64, timeout time.Duration) error { runURL := fmt.Sprintf("%s/application/v4/tenant/%s/application/%s/instance/%s/job/%s-%s/run/%d", - t.apiURL, - t.deployment.Application.Tenant, t.deployment.Application.Application, t.deployment.Application.Instance, - t.deployment.Zone.Environment, t.deployment.Zone.Region, runID) + t.apiOptions.System.URL, + t.deploymentOptions.Deployment.Application.Tenant, t.deploymentOptions.Deployment.Application.Application, t.deploymentOptions.Deployment.Application.Instance, + t.deploymentOptions.Deployment.Zone.Environment, t.deploymentOptions.Deployment.Zone.Region, runID) req, err := http.NewRequest("GET", runURL, nil) if err != nil { return err @@ -403,7 +440,7 @@ func (t *cloudTarget) waitForRun(runID int64, timeout time.Duration) error { q := req.URL.Query() q.Set("after", strconv.FormatInt(lastID, 10)) req.URL.RawQuery = q.Encode() - if err := t.SignRequest(req, t.deployment.Application.SerializedForm()); err != nil { + if err := t.SignRequest(req, t.deploymentOptions.Deployment.Application.SerializedForm()); err != nil { panic(err) } return req @@ -427,7 +464,7 @@ func (t *cloudTarget) waitForRun(runID int64, timeout time.Duration) error { } return true, nil } - _, err = wait(jobSuccessFunc, requestFunc, &t.tlsOptions.KeyPair, timeout) + _, err = wait(jobSuccessFunc, requestFunc, &t.apiOptions.TLSOptions.KeyPair, timeout) return err } @@ -455,14 +492,14 @@ func (t *cloudTarget) printLog(response jobResponse, last int64) int64 { func (t *cloudTarget) discoverEndpoints(timeout time.Duration) error { deploymentURL := fmt.Sprintf("%s/application/v4/tenant/%s/application/%s/instance/%s/environment/%s/region/%s", - t.apiURL, - t.deployment.Application.Tenant, t.deployment.Application.Application, t.deployment.Application.Instance, - t.deployment.Zone.Environment, t.deployment.Zone.Region) + t.apiOptions.System.URL, + t.deploymentOptions.Deployment.Application.Tenant, t.deploymentOptions.Deployment.Application.Application, t.deploymentOptions.Deployment.Application.Instance, + t.deploymentOptions.Deployment.Zone.Environment, t.deploymentOptions.Deployment.Zone.Region) req, err := http.NewRequest("GET", deploymentURL, nil) if err != nil { return err } - if err := t.SignRequest(req, t.deployment.Application.SerializedForm()); err != nil { + if err := t.SignRequest(req, t.deploymentOptions.Deployment.Application.SerializedForm()); err != nil { return err } urlsByCluster := make(map[string]string) @@ -485,13 +522,13 @@ func (t *cloudTarget) discoverEndpoints(timeout time.Duration) error { } return true, nil } - if _, err = wait(endpointFunc, func() *http.Request { return req }, &t.tlsOptions.KeyPair, timeout); err != nil { + if _, err = wait(endpointFunc, func() *http.Request { return req }, &t.apiOptions.TLSOptions.KeyPair, timeout); err != nil { return err } if len(urlsByCluster) == 0 { return fmt.Errorf("no endpoints discovered") } - t.urlsByCluster = urlsByCluster + t.deploymentOptions.ClusterURLs = urlsByCluster return nil } @@ -504,28 +541,26 @@ func isOK(status int) (bool, error) { // LocalTarget creates a target for a Vespa platform running locally. func LocalTarget() Target { - return &customTarget{targetType: localTargetType, baseURL: "http://127.0.0.1"} + return &customTarget{targetType: TargetLocal, baseURL: "http://127.0.0.1"} } // CustomTarget creates a Target for a Vespa platform running at baseURL. func CustomTarget(baseURL string) Target { - return &customTarget{targetType: customTargetType, baseURL: baseURL} + return &customTarget{targetType: TargetCustom, baseURL: baseURL} } -// CloudTarget creates a Target for the Vespa Cloud platform. -func CloudTarget(apiURL string, deployment Deployment, apiKey []byte, tlsOptions TLSOptions, logOptions LogOptions, - authConfigPath string, systemName string, urlsByCluster map[string]string) Target { - return &cloudTarget{ - apiURL: apiURL, - targetType: cloudTargetType, - deployment: deployment, - apiKey: apiKey, - tlsOptions: tlsOptions, - logOptions: logOptions, - authConfigPath: authConfigPath, - systemName: systemName, - urlsByCluster: urlsByCluster, +// CloudTarget creates a Target for the Vespa Cloud or hosted Vespa platform. +func CloudTarget(apiOptions APIOptions, deploymentOptions CloudDeploymentOptions, logOptions LogOptions) (Target, error) { + ztsClient, err := zts.NewClient(zts.DefaultURL, util.ActiveHttpClient) + if err != nil { + return nil, err } + return &cloudTarget{ + apiOptions: apiOptions, + deploymentOptions: deploymentOptions, + logOptions: logOptions, + ztsClient: ztsClient, + }, nil } type deploymentEndpoint struct { @@ -571,7 +606,8 @@ func wait(fn responseFunc, reqFn requestFunc, certificate *tls.Certificate, time deadline := time.Now().Add(timeout) loopOnce := timeout == 0 for time.Now().Before(deadline) || loopOnce { - response, httpErr = util.HttpDo(reqFn(), 10*time.Second, "") + req := reqFn() + response, httpErr = util.HttpDo(req, 10*time.Second, "") if httpErr == nil { statusCode = response.StatusCode body, err := ioutil.ReadAll(response.Body) diff --git a/client/go/vespa/target_test.go b/client/go/vespa/target_test.go index 8391655eaf7..bf3e0fae7d0 100644 --- a/client/go/vespa/target_test.go +++ b/client/go/vespa/target_test.go @@ -169,12 +169,23 @@ func createCloudTarget(t *testing.T, url string, logWriter io.Writer) Target { apiKey, err := CreateAPIKey() assert.Nil(t, err) - target := CloudTarget("https://example.com", Deployment{ - Application: ApplicationID{Tenant: "t1", Application: "a1", Instance: "i1"}, - Zone: ZoneID{Environment: "dev", Region: "us-north-1"}, - }, apiKey, TLSOptions{KeyPair: x509KeyPair}, LogOptions{Writer: logWriter}, "", "", nil) + target, err := CloudTarget( + APIOptions{APIKey: apiKey, System: PublicSystem}, + CloudDeploymentOptions{ + Deployment: Deployment{ + Application: ApplicationID{Tenant: "t1", Application: "a1", Instance: "i1"}, + Zone: ZoneID{Environment: "dev", Region: "us-north-1"}, + }, + TLSOptions: TLSOptions{KeyPair: x509KeyPair}, + }, + LogOptions{Writer: logWriter}, + ) + if err != nil { + t.Fatal(err) + } if ct, ok := target.(*cloudTarget); ok { - ct.apiURL = url + ct.apiOptions.System.URL = url + ct.ztsClient = &mockZTSClient{token: "foo bar"} } else { t.Fatalf("Wrong target type %T", ct) } @@ -195,3 +206,11 @@ func assertServiceWait(t *testing.T, expectedStatus int, target Target, service assert.Nil(t, err) assert.Equal(t, expectedStatus, status) } + +type mockZTSClient struct { + token string +} + +func (c *mockZTSClient) AccessToken(domain string, certificate tls.Certificate) (string, error) { + return c.token, nil +} diff --git a/client/go/vespa/xml/config.go b/client/go/vespa/xml/config.go index c9efcb7f340..c9af92339bc 100644 --- a/client/go/vespa/xml/config.go +++ b/client/go/vespa/xml/config.go @@ -9,6 +9,8 @@ import ( "regexp" "strconv" "strings" + + "github.com/vespa-engine/vespa/client/go/vespa" ) var DefaultDeployment Deployment @@ -218,8 +220,9 @@ func ParseNodeCount(s string) (int, int, error) { } // IsProdRegion returns whether string s is a valid production region. -func IsProdRegion(s string, system string) bool { - if system == "publiccd" { +func IsProdRegion(s string, system vespa.System) bool { + // TODO: Add support for cd and main systems + if system.Name == vespa.PublicCDSystem.Name { return s == "aws-us-east-1c" } switch s { diff --git a/client/go/zts/zts.go b/client/go/zts/zts.go new file mode 100644 index 00000000000..b1a47db8e48 --- /dev/null +++ b/client/go/zts/zts.go @@ -0,0 +1,55 @@ +package zts + +import ( + "crypto/tls" + "encoding/json" + "fmt" + "net/http" + "net/url" + "strings" + "time" + + "github.com/vespa-engine/vespa/client/go/util" +) + +const DefaultURL = "https://zts.athenz.ouroath.com:4443" + +// Client is a client for Athenz ZTS, an authentication token service. +type Client struct { + client util.HttpClient + tokenURL *url.URL +} + +// NewClient creates a new client for an Athenz ZTS service located at serviceURL. +func NewClient(serviceURL string, client util.HttpClient) (*Client, error) { + tokenURL, err := url.Parse(serviceURL) + if err != nil { + return nil, err + } + tokenURL.Path = "/zts/v1/oauth2/token" + return &Client{tokenURL: tokenURL, client: client}, nil +} + +// AccessToken returns an access token within the given domain, using certificate to authenticate with ZTS. +func (c *Client) AccessToken(domain string, certificate tls.Certificate) (string, error) { + data := fmt.Sprintf("grant_type=client_credentials&scope=%s:domain", domain) + req, err := http.NewRequest("POST", c.tokenURL.String(), strings.NewReader(data)) + if err != nil { + return "", err + } + c.client.UseCertificate([]tls.Certificate{certificate}) + response, err := c.client.Do(req, 10*time.Second) + if err != nil { + return "", err + } + defer response.Body.Close() + + var ztsResponse struct { + AccessToken string `json:"access_token"` + } + dec := json.NewDecoder(response.Body) + if err := dec.Decode(&ztsResponse); err != nil { + return "", err + } + return ztsResponse.AccessToken, nil +} diff --git a/client/go/zts/zts_test.go b/client/go/zts/zts_test.go new file mode 100644 index 00000000000..f1bd9c1ba75 --- /dev/null +++ b/client/go/zts/zts_test.go @@ -0,0 +1,25 @@ +package zts + +import ( + "crypto/tls" + "testing" + + "github.com/vespa-engine/vespa/client/go/mock" +) + +func TestAccessToken(t *testing.T) { + httpClient := mock.HTTPClient{} + client, err := NewClient("http://example.com", &httpClient) + if err != nil { + t.Fatal(err) + } + httpClient.NextResponse(200, `{"access_token": "foo bar"}`) + token, err := client.AccessToken("vespa.vespa", tls.Certificate{}) + if err != nil { + t.Fatal(err) + } + want := "foo bar" + if token != want { + t.Errorf("got %q, want %q", token, want) + } +} diff --git a/config-model/src/main/java/com/yahoo/config/model/builder/xml/ConfigModelBuilder.java b/config-model/src/main/java/com/yahoo/config/model/builder/xml/ConfigModelBuilder.java index 24a8d81c754..656f78ba2a9 100644 --- a/config-model/src/main/java/com/yahoo/config/model/builder/xml/ConfigModelBuilder.java +++ b/config-model/src/main/java/com/yahoo/config/model/builder/xml/ConfigModelBuilder.java @@ -80,9 +80,8 @@ public abstract class ConfigModelBuilder<MODEL extends ConfigModel> extends Abst private static String getIdString(Element spec) { String idString = XmlHelper.getIdString(spec); - if (idString == null || idString.isEmpty()) { + if (idString.isEmpty()) idString = spec.getTagName(); - } return idString; } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java b/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java index 6960a0a8afd..26b4b78fcaa 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java @@ -965,12 +965,13 @@ public class RankProfile implements Cloneable { Map<String, RankingExpressionFunction> compiledFunctions = new LinkedHashMap<>(); Map.Entry<String, RankingExpressionFunction> entry; // Compile all functions. Why iterate in such a complicated way? - // Because some functions (imported models adding generated macros) may add other functions during compiling. + // Because some functions (imported models adding generated functions) may add other functions during compiling. // A straightforward iteration will either miss those functions, or may cause a ConcurrentModificationException while (null != (entry = findUncompiledFunction(functions.get(), compiledFunctions.keySet()))) { RankingExpressionFunction rankingExpressionFunction = entry.getValue(); RankingExpressionFunction compiled = compile(rankingExpressionFunction, queryProfiles, featureTypes, - importedModels, getConstants(), inlineFunctions, expressionTransforms); + importedModels, getConstants(), inlineFunctions, + expressionTransforms); compiledFunctions.put(entry.getKey(), compiled); } return compiledFunctions; @@ -986,12 +987,12 @@ public class RankProfile implements Cloneable { } private RankingExpressionFunction compile(RankingExpressionFunction function, - QueryProfileRegistry queryProfiles, - Map<Reference, TensorType> featureTypes, - ImportedMlModels importedModels, - Map<String, Value> constants, - Map<String, RankingExpressionFunction> inlineFunctions, - ExpressionTransforms expressionTransforms) { + QueryProfileRegistry queryProfiles, + Map<Reference, TensorType> featureTypes, + ImportedMlModels importedModels, + Map<String, Value> constants, + Map<String, RankingExpressionFunction> inlineFunctions, + ExpressionTransforms expressionTransforms) { if (function == null) return null; RankProfileTransformContext context = new RankProfileTransformContext(this, diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ConvertSchemaCollection.java b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ConvertSchemaCollection.java new file mode 100644 index 00000000000..9b69a82a8ff --- /dev/null +++ b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ConvertSchemaCollection.java @@ -0,0 +1,292 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.searchdefinition.parser; + +import com.yahoo.document.DataType; +import com.yahoo.document.DocumentType; +import com.yahoo.document.DocumentTypeManager; +import com.yahoo.document.ReferenceDataType; +import com.yahoo.document.StructDataType; +import com.yahoo.document.PositionDataType; +import com.yahoo.document.WeightedSetDataType; +import com.yahoo.document.annotation.AnnotationReferenceDataType; +import com.yahoo.document.annotation.AnnotationType; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +/** + * Class converting a collection of schemas from the intermediate format. + * For now only conversion to DocumentType (with contents). + * + * @author arnej27959 + **/ +public class ConvertSchemaCollection { + + private final IntermediateCollection input; + private final List<ParsedSchema> orderedInput = new ArrayList<>(); + private final DocumentTypeManager docMan; + + public ConvertSchemaCollection(IntermediateCollection input, + DocumentTypeManager documentTypeManager) + { + this.input = input; + this.docMan = documentTypeManager; + order(); + pushTypesToDocuments(); + } + + public void convertTypes() { + convertDataTypes(); + registerDataTypes(); + } + + void order() { + var map = input.getParsedSchemas(); + for (var schema : map.values()) { + findOrdering(schema); + } + } + + void findOrdering(ParsedSchema schema) { + if (orderedInput.contains(schema)) return; + for (var parent : schema.getAllResolvedInherits()) { + findOrdering(parent); + } + orderedInput.add(schema); + } + + void pushTypesToDocuments() { + for (var schema : orderedInput) { + for (var struct : schema.getStructs()) { + schema.getDocument().addStruct(struct); + } + for (var annotation : schema.getAnnotations()) { + schema.getDocument().addAnnotation(annotation); + } + } + } + + Map<String, DocumentType> documentsInProgress = new HashMap<>(); + Map<String, StructDataType> structsInProgress = new HashMap<>(); + Map<String, AnnotationType> annotationsInProgress = new HashMap<>(); + + StructDataType findStructInProgress(String name, ParsedDocument context) { + var resolved = findStructFrom(context, name); + if (resolved == null) { + throw new IllegalArgumentException("no struct named " + name + " in context " + context); + } + String structId = resolved.getOwner() + "->" + resolved.name(); + var struct = structsInProgress.get(structId); + assert(struct != null); + return struct; + } + + AnnotationType findAnnotationInProgress(String name, ParsedDocument context) { + var resolved = findAnnotationFrom(context, name); + String annotationId = resolved.getOwner() + "->" + resolved.name(); + var annotation = annotationsInProgress.get(annotationId); + if (annotation == null) { + throw new IllegalArgumentException("no annotation named " + name + " in context " + context); + } + return annotation; + } + + ParsedStruct findStructFrom(ParsedDocument doc, String name) { + ParsedStruct found = doc.getStruct(name); + if (found != null) return found; + for (var parent : doc.getResolvedInherits()) { + var fromParent = findStructFrom(parent, name); + if (fromParent == null) continue; + if (fromParent == found) continue; + if (found == null) { + found = fromParent; + } else { + throw new IllegalArgumentException("conflicting values for struct " + name + " in " +doc); + } + } + return found; + } + + ParsedAnnotation findAnnotationFrom(ParsedDocument doc, String name) { + ParsedAnnotation found = doc.getAnnotation(name); + if (found != null) return found; + for (var parent : doc.getResolvedInherits()) { + var fromParent = findAnnotationFrom(parent, name); + if (fromParent == null) continue; + if (fromParent == found) continue; + if (found == null) { + found = fromParent; + } else { + throw new IllegalArgumentException("conflicting values for annotation " + name + " in " +doc); + } + } + return found; + } + + private DataType createArray(ParsedType pType, ParsedDocument context) { + DataType nested = resolveType(pType.nestedType(), context); + return DataType.getArray(nested); + } + + private DataType createWset(ParsedType pType, ParsedDocument context) { + DataType nested = resolveType(pType.nestedType(), context); + boolean cine = pType.getCreateIfNonExistent(); + boolean riz = pType.getRemoveIfZero(); + return new WeightedSetDataType(nested, cine, riz); + } + + private DataType createMap(ParsedType pType, ParsedDocument context) { + DataType kt = resolveType(pType.mapKeyType(), context); + DataType vt = resolveType(pType.mapValueType(), context); + return DataType.getMap(kt, vt); + } + + private DocumentType findDocInProgress(String name) { + var dt = documentsInProgress.get(name); + if (dt == null) { + throw new IllegalArgumentException("missing document type for: " + name); + } + return dt; + } + + private DataType createAnnRef(ParsedType pType, ParsedDocument context) { + AnnotationType annotation = findAnnotationInProgress(pType.getNameOfReferencedAnnotation(), context); + return new AnnotationReferenceDataType(annotation); + } + + private DataType createDocRef(ParsedType pType) { + var ref = pType.getReferencedDocumentType(); + assert(ref.getVariant() == ParsedType.Variant.DOCUMENT); + return ReferenceDataType.createWithInferredId(findDocInProgress(ref.name())); + } + + DataType resolveType(ParsedType pType, ParsedDocument context) { + switch (pType.getVariant()) { + case NONE: return DataType.NONE; + case BUILTIN: return docMan.getDataType(pType.name()); + case POSITION: return PositionDataType.INSTANCE; + case ARRAY: return createArray(pType, context); + case WSET: return createWset(pType, context); + case MAP: return createMap(pType, context); + case TENSOR: return DataType.getTensor(pType.getTensorType()); + case DOC_REFERENCE: return createDocRef(pType); + case ANN_REFERENCE: return createAnnRef(pType, context); + case DOCUMENT: return findDocInProgress(pType.name()); + case STRUCT: return findStructInProgress(pType.name(), context); + case UNKNOWN: + // fallthrough + } + // unknown is probably struct, but could be document: + if (documentsInProgress.containsKey(pType.name())) { + pType.setVariant(ParsedType.Variant.DOCUMENT); + return findDocInProgress(pType.name()); + } + var struct = findStructInProgress(pType.name(), context); + pType.setVariant(ParsedType.Variant.STRUCT); + return struct; + } + + void convertDataTypes() { + for (var schema : orderedInput) { + String name = schema.getDocument().name(); + documentsInProgress.put(name, new DocumentType(name)); + } + for (var schema : orderedInput) { + var doc = schema.getDocument(); + for (var struct : doc.getStructs()) { + var dt = new StructDataType(struct.name()); + String structId = doc.name() + "->" + struct.name(); + structsInProgress.put(structId, dt); + } + for (var annotation : doc.getAnnotations()) { + String annId = doc.name() + "->" + annotation.name(); + var at = new AnnotationType(annotation.name()); + annotationsInProgress.put(annId, at); + var withStruct = annotation.getStruct(); + if (withStruct.isPresent()) { + var sn = withStruct.get().name(); + var dt = new StructDataType(sn); + String structId = doc.name() + "->" + sn; + structsInProgress.put(structId, dt); + } + } + } + for (var schema : orderedInput) { + var doc = schema.getDocument(); + for (var struct : doc.getStructs()) { + String structId = doc.name() + "->" + struct.name(); + var toFill = structsInProgress.get(structId); + for (String inherit : struct.getInherited()) { + var parent = findStructInProgress(inherit, doc); + toFill.inherit(parent); + } + for (ParsedField field : struct.getFields()) { + var t = resolveType(field.getType(), doc); + var f = new com.yahoo.document.Field(field.name(), t); + toFill.addField(f); + } + } + for (var annotation : doc.getAnnotations()) { + String annId = doc.name() + "->" + annotation.name(); + var at = annotationsInProgress.get(annId); + var withStruct = annotation.getStruct(); + if (withStruct.isPresent()) { + ParsedStruct struct = withStruct.get(); + String structId = doc.name() + "->" + struct.name(); + var toFill = structsInProgress.get(structId); + for (ParsedField field : struct.getFields()) { + var t = resolveType(field.getType(), doc); + var f = new com.yahoo.document.Field(field.name(), t); + toFill.addField(f); + } + at.setDataType(toFill); + } + for (String inherit : annotation.getInherited()) { + var parent = findAnnotationInProgress(inherit, doc); + at.inherit(parent); + } + } + + var docToFill = documentsInProgress.get(doc.name()); + Map<String, Collection<String>> fieldSets = new HashMap<>(); + List<String> inDocFields = new ArrayList<>(); + for (var docField : doc.getFields()) { + String name = docField.name(); + var t = resolveType(docField.getType(), doc); + var f = new com.yahoo.document.Field(name, t); + docToFill.addField(f); + inDocFields.add(name); + } + fieldSets.put("[document]", inDocFields); + for (var extraField : schema.getFields()) { + String name = extraField.name(); + var t = resolveType(extraField.getType(), doc); + var f = new com.yahoo.document.Field(name, t); + docToFill.addField(f); + } + for (var fieldset : schema.getFieldSets()) { + fieldSets.put(fieldset.name(), fieldset.getFieldNames()); + } + docToFill.addFieldSets(fieldSets); + for (String inherit : doc.getInherited()) { + docToFill.inherit(findDocInProgress(inherit)); + } + } + } + + void registerDataTypes() { + for (DataType t : structsInProgress.values()) { + docMan.register(t); + } + for (DocumentType t : documentsInProgress.values()) { + docMan.register(t); + } + for (AnnotationType t : annotationsInProgress.values()) { + docMan.getAnnotationTypeRegistry().register(t); + } + } + +} diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/parser/InheritanceResolver.java b/config-model/src/main/java/com/yahoo/searchdefinition/parser/InheritanceResolver.java index 488464ccd1f..edcbf85b5dc 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/parser/InheritanceResolver.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/parser/InheritanceResolver.java @@ -28,7 +28,7 @@ public class InheritanceResolver { String.join(" -> ", seen)); } seen.add(name); - for (ParsedSchema parent : schema.getResolvedInherits()) { + for (ParsedSchema parent : schema.getAllResolvedInherits()) { inheritanceCycleCheck(parent, seen); } seen.remove(name); @@ -57,11 +57,23 @@ public class InheritanceResolver { for (ParsedSchema schema : parsedSchemas.values()) { if (! schema.hasDocument()) { // TODO: is schema without a document even valid? - continue; + // could make sense for schemas with just rank-profile functions + // it makes life easier to behave as if there was en empty + // document block here. + var doc = new ParsedDocument(schema.name()); + for (String inherit : schema.getInherited()) { + doc.inherit(inherit); + } + schema.addDocument(doc); } ParsedDocument doc = schema.getDocument(); var old = parsedDocs.put(doc.name(), doc); - assert(old == null); + if (old != null) { + throw new IllegalArgumentException("duplicate document declaration for " + doc.name()); + } + for (String docInherit : doc.getInherited()) { + schema.inheritByDocument(docInherit); + } } for (ParsedDocument doc : parsedDocs.values()) { for (String inherit : doc.getInherited()) { @@ -72,6 +84,14 @@ public class InheritanceResolver { doc.resolveInherit(inherit, parentDoc); } } + for (ParsedSchema schema : parsedSchemas.values()) { + for (String inherit : schema.getInheritedByDocument()) { + var parent = parsedSchemas.get(inherit); + assert(parent.hasDocument()); + assert(parent.getDocument().name().equals(inherit)); + schema.resolveInheritByDocument(inherit, parent); + } + } } private void inheritanceCycleCheck(ParsedDocument document, List<String> seen) { @@ -98,8 +118,8 @@ public class InheritanceResolver { public void resolveInheritance() { resolveSchemaInheritance(); resolveDocumentInheritance(); - checkSchemaCycles(); checkDocumentCycles(); + checkSchemaCycles(); } } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedAnnotation.java b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedAnnotation.java index a4e38795850..f22d370b1d8 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedAnnotation.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedAnnotation.java @@ -14,6 +14,7 @@ class ParsedAnnotation extends ParsedBlock { private ParsedStruct wrappedStruct = null; private final List<String> inherited = new ArrayList<>(); + private String ownedBy = null; ParsedAnnotation(String name) { super(name, "annotation"); @@ -21,7 +22,12 @@ class ParsedAnnotation extends ParsedBlock { public List<String> getInherited() { return List.copyOf(inherited); } public Optional<ParsedStruct> getStruct() { return Optional.ofNullable(wrappedStruct); } + public String getOwner() { return ownedBy; } void setStruct(ParsedStruct struct) { this.wrappedStruct = struct; } void inherit(String other) { inherited.add(other); } + void tagOwner(String owner) { + verifyThat(ownedBy == null, "already owned by", ownedBy); + this.ownedBy = owner; + } } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedDocument.java b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedDocument.java index 8cd64ef16f7..dd61124c3a7 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedDocument.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedDocument.java @@ -31,6 +31,8 @@ public class ParsedDocument extends ParsedBlock { List<ParsedDocument> getResolvedInherits() { return List.copyOf(resolvedInherits.values()); } List<ParsedField> getFields() { return List.copyOf(docFields.values()); } List<ParsedStruct> getStructs() { return List.copyOf(docStructs.values()); } + ParsedStruct getStruct(String name) { return docStructs.get(name); } + ParsedAnnotation getAnnotation(String name) { return docAnnotations.get(name); } void inherit(String other) { inherited.add(other); } @@ -44,12 +46,14 @@ public class ParsedDocument extends ParsedBlock { String sName = struct.name(); verifyThat(! docStructs.containsKey(sName), "already has struct", sName); docStructs.put(sName, struct); + struct.tagOwner(name()); } void addAnnotation(ParsedAnnotation annotation) { String annName = annotation.name(); verifyThat(! docAnnotations.containsKey(annName), "already has annotation", annName); docAnnotations.put(annName, annotation); + annotation.tagOwner(name()); } public String toString() { return "document " + name(); } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedSchema.java b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedSchema.java index bf448b31dd2..a0b238f1f43 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedSchema.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedSchema.java @@ -36,7 +36,9 @@ public class ParsedSchema extends ParsedBlock { private final List<OnnxModel> onnxModels = new ArrayList<>(); private final List<RankingConstant> rankingConstants = new ArrayList<>(); private final List<String> inherited = new ArrayList<>(); + private final List<String> inheritedByDocument = new ArrayList<>(); private final Map<String, ParsedSchema> resolvedInherits = new HashMap(); + private final Map<String, ParsedSchema> allResolvedInherits = new HashMap(); private final Map<String, ParsedAnnotation> extraAnnotations = new HashMap<>(); private final Map<String, ParsedDocumentSummary> docSums = new HashMap<>(); private final Map<String, ParsedField> extraFields = new HashMap<>(); @@ -64,8 +66,10 @@ public class ParsedSchema extends ParsedBlock { List<ParsedStruct> getStructs() { return List.copyOf(extraStructs.values()); } List<RankingConstant> getRankingConstants() { return List.copyOf(rankingConstants); } List<String> getInherited() { return List.copyOf(inherited); } + List<String> getInheritedByDocument() { return List.copyOf(inheritedByDocument); } Map<String, ParsedRankProfile> getRankProfiles() { return Map.copyOf(rankProfiles); } List<ParsedSchema> getResolvedInherits() { return List.copyOf(resolvedInherits.values()); } + List<ParsedSchema> getAllResolvedInherits() { return List.copyOf(allResolvedInherits.values()); } void addAnnotation(ParsedAnnotation annotation) { String annName = annotation.name(); @@ -76,6 +80,8 @@ public class ParsedSchema extends ParsedBlock { void addDocument(ParsedDocument document) { verifyThat(myDocument == null, "already has", myDocument, "so cannot add", document); + verifyThat(name().equals(document.name()), + "schema " + name() + "can only contain document named " + name() + ", was: "+ document.name()); this.myDocument = document; } @@ -133,6 +139,8 @@ public class ParsedSchema extends ParsedBlock { void inherit(String other) { inherited.add(other); } + void inheritByDocument(String other) { inheritedByDocument.add(other); } + void setStemming(Stemming value) { verifyThat((defaultStemming == null) || (defaultStemming == value), "already has stemming", defaultStemming, "cannot also set", value); @@ -144,6 +152,16 @@ public class ParsedSchema extends ParsedBlock { verifyThat(name.equals(parsed.name()), "resolveInherit name mismatch for", name); verifyThat(! resolvedInherits.containsKey(name), "double resolveInherit for", name); resolvedInherits.put(name, parsed); + var old = allResolvedInherits.put(name, parsed); + verifyThat(old == null || old == parsed, "conflicting resolveInherit for", name); + } + + void resolveInheritByDocument(String name, ParsedSchema parsed) { + verifyThat(inheritedByDocument.contains(name), + "resolveInheritByDocument for non-inherited name", name); + verifyThat(name.equals(parsed.name()), "resolveInheritByDocument name mismatch for", name); + var old = allResolvedInherits.put(name, parsed); + verifyThat(old == null || old == parsed, "conflicting resolveInherit for", name); } } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedStruct.java b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedStruct.java index 67f2f137bc1..cc3b2425726 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedStruct.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedStruct.java @@ -15,6 +15,7 @@ import java.util.Map; public class ParsedStruct extends ParsedBlock { private final List<String> inherited = new ArrayList<>(); private final Map<String, ParsedField> fields = new HashMap<>(); + private String ownedBy = null; public ParsedStruct(String name) { super(name, "struct"); @@ -22,6 +23,7 @@ public class ParsedStruct extends ParsedBlock { List<ParsedField> getFields() { return List.copyOf(fields.values()); } List<String> getInherited() { return List.copyOf(inherited); } + String getOwner() { return ownedBy; } void addField(ParsedField field) { String fieldName = field.name(); @@ -29,6 +31,15 @@ public class ParsedStruct extends ParsedBlock { fields.put(fieldName, field); } - void inherit(String other) { inherited.add(other); } + void inherit(String other) { + verifyThat(! name().equals(other), "cannot inherit from itself"); + inherited.add(other); + } + + void tagOwner(String document) { + verifyThat(ownedBy == null, "already owned by document "+ownedBy); + this.ownedBy = document; + } + } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedType.java b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedType.java index 9f02c5247ef..d3e85bc1b11 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedType.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedType.java @@ -13,10 +13,9 @@ import com.yahoo.tensor.TensorType; class ParsedType { public enum Variant { NONE, - BOOL, BYTE, INT, LONG, - STRING, - FLOAT, DOUBLE, - URI, PREDICATE, TENSOR, + BUILTIN, + POSITION, + TENSOR, ARRAY, WSET, MAP, DOC_REFERENCE, ANN_REFERENCE, @@ -35,16 +34,16 @@ class ParsedType { private static Variant guessVariant(String name) { switch (name) { - case "bool": return Variant.BOOL; - case "byte": return Variant.BYTE; - case "int": return Variant.INT; - case "long": return Variant.LONG; - case "string": return Variant.STRING; - case "float": return Variant.FLOAT; - case "double": return Variant.DOUBLE; - case "uri": return Variant.URI; - case "predicate": return Variant.PREDICATE; - case "position": return Variant.STRUCT; + case "bool": return Variant.BUILTIN; + case "byte": return Variant.BUILTIN; + case "int": return Variant.BUILTIN; + case "long": return Variant.BUILTIN; + case "string": return Variant.BUILTIN; + case "float": return Variant.BUILTIN; + case "double": return Variant.BUILTIN; + case "uri": return Variant.BUILTIN; + case "predicate": return Variant.BUILTIN; + case "position": return Variant.POSITION; } return Variant.UNKNOWN; } @@ -53,20 +52,28 @@ class ParsedType { public Variant getVariant() { return variant; } public ParsedType mapKeyType() { assert(variant == Variant.MAP); return keyType; } public ParsedType mapValueType() { assert(variant == Variant.MAP); return valType; } - public ParsedType nestedType() { assert(variant == Variant.ARRAY || variant == Variant.WSET); return valType; } + public ParsedType nestedType() { assert(variant == Variant.ARRAY || variant == Variant.WSET); assert(valType != null); return valType; } public boolean getCreateIfNonExistent() { assert(variant == Variant.WSET); return this.createIfNonExistent; } public boolean getRemoveIfZero() { assert(variant == Variant.WSET); return this.removeIfZero; } public ParsedType getReferencedDocumentType() { assert(variant == Variant.DOC_REFERENCE); return valType; } public TensorType getTensorType() { assert(variant == Variant.TENSOR); return tensorType; } + public String getNameOfReferencedAnnotation() { + assert(variant == Variant.ANN_REFERENCE); + String prefix = "annotationreference<"; + int fromPos = prefix.length(); + int toPos = name.length() - 1; + return name.substring(fromPos, toPos); + } + private ParsedType(String name, Variant variant) { this(name, variant, null, null, null); } private ParsedType(String name, Variant variant, ParsedType vt) { - this(name, variant, vt, null, null); + this(name, variant, null, vt, null); } private ParsedType(String name, Variant variant, ParsedType kt, ParsedType vt) { - this(name, variant, vt, kt, null); + this(name, variant, kt, vt, null); } private ParsedType(String name, Variant variant, ParsedType kt, ParsedType vt, TensorType tType) { this.name = name; @@ -77,22 +84,28 @@ class ParsedType { } static ParsedType mapType(ParsedType kt, ParsedType vt) { + assert(kt != null); + assert(vt != null); String name = "map<" + kt.name() + "," + vt.name() + ">"; return new ParsedType(name, Variant.MAP, kt, vt); } static ParsedType arrayOf(ParsedType vt) { + assert(vt != null); return new ParsedType("array<" + vt.name() + ">", Variant.ARRAY, vt); } static ParsedType wsetOf(ParsedType vt) { + assert(vt != null); return new ParsedType("weightedset<" + vt.name() + ">", Variant.WSET, vt); } static ParsedType documentRef(ParsedType docType) { + assert(docType != null); return new ParsedType("reference<" + docType.name + ">", Variant.DOC_REFERENCE, docType); } static ParsedType annotationRef(String name) { return new ParsedType("annotationreference<" + name + ">", Variant.ANN_REFERENCE); } static ParsedType tensorType(TensorType tType) { + assert(tType != null); return new ParsedType(tType.toString(), Variant.TENSOR, null, null, tType); } static ParsedType fromName(String name) { diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java index c88d225f527..17690c6ecab 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java @@ -824,6 +824,8 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { .dockerImageRepository(deployState.getWantedDockerImageRepo()) .build(); int nodeCount = deployState.zone().environment().isProduction() ? 2 : 1; + deployState.getDeployLogger().logApplicationPackage(Level.INFO, "Using " + nodeCount + + " nodes in " + cluster); Capacity capacity = Capacity.from(new ClusterResources(nodeCount, 1, NodeResources.unspecified()), false, !deployState.getProperties().isBootstrap()); diff --git a/config-model/src/main/javacc/IntermediateParser.jj b/config-model/src/main/javacc/IntermediateParser.jj index cc03773f333..a8e77dead6e 100644 --- a/config-model/src/main/javacc/IntermediateParser.jj +++ b/config-model/src/main/javacc/IntermediateParser.jj @@ -427,16 +427,11 @@ void rootSchemaItem(ParsedSchema schema) : { } */ ParsedSchema rootDocument() : { - ParsedSchema schema = new ParsedSchema("<unnamed>"); + ParsedSchema schema = null; } { - ( (rootDocumentItem(schema) (<NL>)*)*<EOF> ) + ( (schema = rootDocumentItem(schema) (<NL>)*)*<EOF> ) { - if (schema.hasDocument()) { - ParsedDocument doc = schema.getDocument(); - schema = new ParsedSchema(doc.name()); - schema.addDocument(doc); - } return schema; } } @@ -446,9 +441,16 @@ ParsedSchema rootDocument() : * * @param schema the schema object to modify. */ -void rootDocumentItem(ParsedSchema schema) : { } +ParsedSchema rootDocumentItem(ParsedSchema schema) : +{ + ParsedDocument doc = null; +} { - ( namedDocument(schema) ) + ( doc = namedDocument() { + if (schema == null) schema = new ParsedSchema(doc.name()); + schema.addDocument(doc); + return schema; + } ) } /** @@ -483,10 +485,8 @@ void document(ParsedSchema schema) : /** * Consumes a document element, explicitly named - * - * @param schema the schema object to add content to */ -void namedDocument(ParsedSchema schema) : +ParsedDocument namedDocument() : { String name; ParsedDocument document; @@ -496,7 +496,7 @@ void namedDocument(ParsedSchema schema) : [ inheritsDocument(document) (<NL>)* ] <LBRACE> (<NL>)* (documentBody(document) (<NL>)*)* <RBRACE> ) { - schema.addDocument(document); + return document; } } diff --git a/config-model/src/test/converter/child.sd b/config-model/src/test/converter/child.sd new file mode 100644 index 00000000000..cdfc339ed59 --- /dev/null +++ b/config-model/src/test/converter/child.sd @@ -0,0 +1,23 @@ +# Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +search child { + + document child inherits parent { + + field a type uri { + indexing: index | summary + } + + field r type redef { + indexing: summary + } + + field aaa type annotationreference<gpa> { } + + field modelref type reference<other> { } + + } + field outrarr type array<string> { + indexing: input a | to_array | summary + } + +} diff --git a/config-model/src/test/converter/grandparent.sd b/config-model/src/test/converter/grandparent.sd new file mode 100644 index 00000000000..603553f739d --- /dev/null +++ b/config-model/src/test/converter/grandparent.sd @@ -0,0 +1,32 @@ +# Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +search grandparent { + + struct item { + field f1i type int {} + } + + struct gps { + field reftoa type annotationreference<gpa> {} + field someitems type array<item> {} + } + + document grandparent { + + field c type map<string, gps> { + indexing: index + } + + #field inrgp type redef { + #} + } + + annotation gpa { + field city type string {} + field zip type int {} + } + + #struct redef { + # field y type int {} + # field z type string {} + #} +} diff --git a/config-model/src/test/converter/other.sd b/config-model/src/test/converter/other.sd new file mode 100644 index 00000000000..6cf2a56c43a --- /dev/null +++ b/config-model/src/test/converter/other.sd @@ -0,0 +1,16 @@ +# Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +search other { + + document other { + + field c type tensor(d[512]) { + indexing: attribute + } + + field d type tensor<float>(cat{},x[13]) { + indexing: attribute + } + + } + +} diff --git a/config-model/src/test/converter/parent.sd b/config-model/src/test/converter/parent.sd new file mode 100644 index 00000000000..f05edaef787 --- /dev/null +++ b/config-model/src/test/converter/parent.sd @@ -0,0 +1,29 @@ +# Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +search parent { + + struct ps { + field wil type weightedset<string> {} + field after type array<long> {} + field psi type item {} + } + + document parent inherits grandparent { + + field b type string { + indexing: index | summary + } + + field bps type ps { + indexing: summary + } + + field location type array<position> { + indexing: attribute + } + } + + struct redef { + field x type int {} + field y type string {} + } +} diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/parser/ConvertIntermediateTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/parser/ConvertIntermediateTestCase.java new file mode 100644 index 00000000000..264481cb3ec --- /dev/null +++ b/config-model/src/test/java/com/yahoo/searchdefinition/parser/ConvertIntermediateTestCase.java @@ -0,0 +1,100 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.searchdefinition.parser; + +import com.yahoo.document.DataType; +import com.yahoo.document.DocumentType; +import com.yahoo.document.DocumentTypeManager; +import static com.yahoo.config.model.test.TestUtil.joinLines; + +import org.junit.Test; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertThrows; + +/** + * @author arnej + */ +public class ConvertIntermediateTestCase { + + @Test + public void can_convert_minimal_schema() throws Exception { + String input = joinLines + ("schema foo {", + " document foo {", + " }", + "}"); + var collection = new IntermediateCollection(); + ParsedSchema schema = collection.addSchemaFromString(input); + assertEquals("foo", schema.getDocument().name()); + collection.resolveInternalConnections(); + var docMan = new DocumentTypeManager(); + var converter = new ConvertSchemaCollection(collection, docMan); + converter.convertTypes(); + var dt = docMan.getDocumentType("foo"); + assertTrue(dt != null); + } + + @Test + public void can_convert_schema_files() throws Exception { + var collection = new IntermediateCollection(); + collection.addSchemaFromFile("src/test/derived/deriver/child.sd"); + collection.addSchemaFromFile("src/test/derived/deriver/grandparent.sd"); + collection.addSchemaFromFile("src/test/derived/deriver/parent.sd"); + assertEquals(collection.getParsedSchemas().size(), 3); + collection.resolveInternalConnections(); + var docMan = new DocumentTypeManager(); + var converter = new ConvertSchemaCollection(collection, docMan); + converter.convertTypes(); + var dt = docMan.getDocumentType("child"); + assertTrue(dt != null); + dt = docMan.getDocumentType("parent"); + assertTrue(dt != null); + dt = docMan.getDocumentType("grandparent"); + assertTrue(dt != null); + } + + @Test + public void can_convert_structs_and_annotations() throws Exception { + var collection = new IntermediateCollection(); + collection.addSchemaFromFile("src/test/converter/child.sd"); + collection.addSchemaFromFile("src/test/converter/other.sd"); + collection.addSchemaFromFile("src/test/converter/parent.sd"); + collection.addSchemaFromFile("src/test/converter/grandparent.sd"); + collection.resolveInternalConnections(); + var docMan = new DocumentTypeManager(); + var converter = new ConvertSchemaCollection(collection, docMan); + converter.convertTypes(); + var dt = docMan.getDocumentType("child"); + assertTrue(dt != null); + for (var parent : dt.getInheritedTypes()) { + System.err.println("dt "+dt.getName()+" inherits from "+parent.getName()); + } + for (var field : dt.fieldSetAll()) { + System.err.println("dt "+dt.getName()+" contains field "+field.getName()+" of type "+field.getDataType()); + } + dt = docMan.getDocumentType("parent"); + assertTrue(dt != null); + for (var parent : dt.getInheritedTypes()) { + System.err.println("dt "+dt.getName()+" inherits from "+parent.getName()); + } + for (var field : dt.fieldSetAll()) { + System.err.println("dt "+dt.getName()+" contains field "+field.getName()+" of type "+field.getDataType()); + } + dt = docMan.getDocumentType("grandparent"); + assertTrue(dt != null); + for (var parent : dt.getInheritedTypes()) { + System.err.println("dt "+dt.getName()+" inherits from "+parent.getName()); + } + for (var field : dt.fieldSetAll()) { + System.err.println("dt "+dt.getName()+" contains field "+field.getName()+" of type "+field.getDataType()); + } + dt = docMan.getDocumentType("other"); + assertTrue(dt != null); + for (var parent : dt.getInheritedTypes()) { + System.err.println("dt "+dt.getName()+" inherits from "+parent.getName()); + } + for (var field : dt.fieldSetAll()) { + System.err.println("dt "+dt.getName()+" contains field "+field.getName()+" of type "+field.getDataType()); + } + } +} diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/parser/IntermediateCollectionTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/parser/IntermediateCollectionTestCase.java index 1ee7ae4937a..da5d0da146f 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/parser/IntermediateCollectionTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/parser/IntermediateCollectionTestCase.java @@ -24,14 +24,14 @@ public class IntermediateCollectionTestCase { public void can_add_minimal_schema() throws Exception { String input = joinLines ("schema foo {", - " document bar {", + " document foo {", " }", "}"); var collection = new IntermediateCollection(); ParsedSchema schema = collection.addSchemaFromString(input); assertEquals("foo", schema.name()); assertTrue(schema.hasDocument()); - assertEquals("bar", schema.getDocument().name()); + assertEquals("foo", schema.getDocument().name()); } @Test @@ -153,9 +153,9 @@ public class IntermediateCollectionTestCase { @Test public void can_detect_schema_inheritance_cycles() throws Exception { var collection = new IntermediateCollection(); - collection.addSchemaFromString("schema foo inherits bar {}"); - collection.addSchemaFromString("schema bar inherits qux {}"); - collection.addSchemaFromString("schema qux inherits foo {}"); + collection.addSchemaFromString("schema foo inherits bar { document foo {} }"); + collection.addSchemaFromString("schema bar inherits qux { document bar {} }"); + collection.addSchemaFromString("schema qux inherits foo { document qux {} }"); assertEquals(collection.getParsedSchemas().size(), 3); var ex = assertThrows(IllegalArgumentException.class, () -> collection.resolveInternalConnections()); @@ -171,6 +171,7 @@ public class IntermediateCollectionTestCase { assertEquals(collection.getParsedSchemas().size(), 3); var ex = assertThrows(IllegalArgumentException.class, () -> collection.resolveInternalConnections()); + System.err.println("ex: "+ex.getMessage()); assertTrue(ex.getMessage().startsWith("Inheritance cycle for documents: ")); } diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/parser/IntermediateParserTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/parser/IntermediateParserTestCase.java index 4379976ce64..6f04b37bb5b 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/parser/IntermediateParserTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/parser/IntermediateParserTestCase.java @@ -35,13 +35,13 @@ public class IntermediateParserTestCase { public void minimal_schema_can_be_parsed() throws Exception { String input = joinLines ("schema foo {", - " document bar {", + " document foo {", " }", "}"); ParsedSchema schema = parseString(input); assertEquals("foo", schema.name()); assertTrue(schema.hasDocument()); - assertEquals("bar", schema.getDocument().name()); + assertEquals("foo", schema.getDocument().name()); } @Test @@ -59,13 +59,13 @@ public class IntermediateParserTestCase { public void multiple_documents_disallowed() throws Exception { String input = joinLines ("schema foo {", - " document foo1 {", + " document foo {", " }", " document foo2 {", " }", "}"); var e = assertThrows(IllegalArgumentException.class, () -> parseString(input)); - assertEquals("schema 'foo' error: already has document foo1 so cannot add document foo2", e.getMessage()); + assertEquals("schema 'foo' error: already has document foo so cannot add document foo2", e.getMessage()); } void checkFileParses(String fileName) throws Exception { diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/NodeFlavors.java b/config-provisioning/src/main/java/com/yahoo/config/provision/NodeFlavors.java index 75a8523d763..a0de085ef96 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/NodeFlavors.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/NodeFlavors.java @@ -6,9 +6,6 @@ import com.yahoo.config.provisioning.FlavorsConfig; import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; -import java.util.Comparator; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; @@ -63,4 +60,9 @@ public class NodeFlavors { return config.flavor().stream().map(Flavor::new).collect(Collectors.toList()); } + @Override + public String toString() { + return String.join(",", configuredFlavors.keySet()); + } + } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/EndpointCertificateDetails.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/EndpointCertificateDetails.java new file mode 100644 index 00000000000..bf1c9333e84 --- /dev/null +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/EndpointCertificateDetails.java @@ -0,0 +1,224 @@ +// 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.integration.certificates; + +import java.util.List; +import java.util.Objects; +import java.util.StringJoiner; + +/** + * This class is used when requesting additional metadata about an application's endpoint certificate from the provider. + * + * @author andreer + */ +public class EndpointCertificateDetails { + + private final String request_id; + private final String requestor; + private final String status; + private final String ticket_id; + private final String athenz_domain; + private final List<EndpointCertificateRequestMetadata.DnsNameStatus> dnsnames; + private final String duration_sec; + private final String expiry; + private final String private_key_kgname; + private final String private_key_keyname; + private final String private_key_version; + private final String cert_key_kgname; + private final String cert_key_keyname; + private final String cert_key_version; + private final String create_time; + private final boolean expiry_protection; + private final String public_key_algo; + private final String issuer; + private final String serial; + + public EndpointCertificateDetails(String request_id, + String requestor, + String status, + String ticket_id, + String athenz_domain, + List<EndpointCertificateRequestMetadata.DnsNameStatus> dnsnames, + String duration_sec, + String expiry, + String private_key_kgname, + String private_key_keyname, + String private_key_version, + String cert_key_kgname, + String cert_key_keyname, + String cert_key_version, + String create_time, + boolean expiry_protection, + String public_key_algo, + String issuer, + String serial) { + this.request_id = request_id; + this.requestor = requestor; + this.status = status; + this.ticket_id = ticket_id; + this.athenz_domain = athenz_domain; + this.dnsnames = dnsnames; + this.duration_sec = duration_sec; + this.expiry = expiry; + this.private_key_kgname = private_key_kgname; + this.private_key_keyname = private_key_keyname; + this.private_key_version = private_key_version; + this.cert_key_kgname = cert_key_kgname; + this.cert_key_keyname = cert_key_keyname; + this.cert_key_version = cert_key_version; + this.create_time = create_time; + this.expiry_protection = expiry_protection; + this.public_key_algo = public_key_algo; + this.issuer = issuer; + this.serial = serial; + } + + public String request_id() { + return request_id; + } + + public String requestor() { + return requestor; + } + + public String status() { + return status; + } + + public String ticket_id() { + return ticket_id; + } + + public String athenz_domain() { + return athenz_domain; + } + + public List<EndpointCertificateRequestMetadata.DnsNameStatus> dnsnames() { + return dnsnames; + } + + public String duration_sec() { + return duration_sec; + } + + public String expiry() { + return expiry; + } + + public String private_key_kgname() { + return private_key_kgname; + } + + public String private_key_keyname() { + return private_key_keyname; + } + + public String private_key_version() { + return private_key_version; + } + + public String cert_key_kgname() { + return cert_key_kgname; + } + + public String cert_key_keyname() { + return cert_key_keyname; + } + + public String cert_key_version() { + return cert_key_version; + } + + public String create_time() { + return create_time; + } + + public boolean expiry_protection() { + return expiry_protection; + } + + public String public_key_algo() { + return public_key_algo; + } + + public String issuer() { + return issuer; + } + + public String serial() { + return serial; + } + + @Override + public String toString() { + return new StringJoiner(", ", EndpointCertificateDetails.class.getSimpleName() + "[", "]") + .add("request_id='" + request_id + "'") + .add("requestor='" + requestor + "'") + .add("status='" + status + "'") + .add("ticket_id='" + ticket_id + "'") + .add("athenz_domain='" + athenz_domain + "'") + .add("dnsnames=" + dnsnames) + .add("duration_sec='" + duration_sec + "'") + .add("expiry='" + expiry + "'") + .add("private_key_kgname='" + private_key_kgname + "'") + .add("private_key_keyname='" + private_key_keyname + "'") + .add("private_key_version='" + private_key_version + "'") + .add("cert_key_kgname='" + cert_key_kgname + "'") + .add("cert_key_keyname='" + cert_key_keyname + "'") + .add("cert_key_version='" + cert_key_version + "'") + .add("create_time='" + create_time + "'") + .add("expiry_protection=" + expiry_protection) + .add("public_key_algo='" + public_key_algo + "'") + .add("issuer='" + issuer + "'") + .add("serial='" + serial + "'") + .toString(); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + EndpointCertificateDetails that = (EndpointCertificateDetails) o; + return expiry_protection == that.expiry_protection + && request_id.equals(that.request_id) + && requestor.equals(that.requestor) + && status.equals(that.status) + && ticket_id.equals(that.ticket_id) + && athenz_domain.equals(that.athenz_domain) + && dnsnames.equals(that.dnsnames) + && duration_sec.equals(that.duration_sec) + && expiry.equals(that.expiry) + && private_key_kgname.equals(that.private_key_kgname) + && private_key_keyname.equals(that.private_key_keyname) + && private_key_version.equals(that.private_key_version) + && cert_key_kgname.equals(that.cert_key_kgname) + && cert_key_keyname.equals(that.cert_key_keyname) + && cert_key_version.equals(that.cert_key_version) + && create_time.equals(that.create_time) + && public_key_algo.equals(that.public_key_algo) + && issuer.equals(that.issuer) + && serial.equals(that.serial); + } + + @Override + public int hashCode() { + return Objects.hash(request_id, + requestor, + status, + ticket_id, + athenz_domain, + dnsnames, + duration_sec, + expiry, + private_key_kgname, + private_key_keyname, + private_key_version, + cert_key_kgname, + cert_key_keyname, + cert_key_version, + create_time, + expiry_protection, + public_key_algo, + issuer, + serial); + } +} diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/EndpointCertificateMetadata.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/EndpointCertificateMetadata.java index 12ff5388eb1..1b36a573bf1 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/EndpointCertificateMetadata.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/EndpointCertificateMetadata.java @@ -18,18 +18,20 @@ public class EndpointCertificateMetadata { private final String certName; private final int version; private final long lastRequested; - private final String requestId; + private final String rootRequestId; + private final Optional<String> leafRequestId; private final List<String> requestedDnsSans; private final String issuer; private final Optional<Long> expiry; private final Optional<Long> lastRefreshed; - public EndpointCertificateMetadata(String keyName, String certName, int version, long lastRequested, String requestId, List<String> requestedDnsSans, String issuer, Optional<Long> expiry, Optional<Long> lastRefreshed) { + public EndpointCertificateMetadata(String keyName, String certName, int version, long lastRequested, String rootRequestId, Optional<String> leafRequestId, List<String> requestedDnsSans, String issuer, Optional<Long> expiry, Optional<Long> lastRefreshed) { this.keyName = keyName; this.certName = certName; this.version = version; this.lastRequested = lastRequested; - this.requestId = requestId; + this.rootRequestId = rootRequestId; + this.leafRequestId = leafRequestId; this.requestedDnsSans = requestedDnsSans; this.issuer = issuer; this.expiry = expiry; @@ -52,8 +54,18 @@ public class EndpointCertificateMetadata { return lastRequested; } - public String requestId() { - return requestId; + /** + * @return The request id of the first request made for this certificate. Should not change. + */ + public String rootRequestId() { + return rootRequestId; + } + + /** + * @return The request id of the last known request made for this certificate. Changes on refresh, may be outdated! + */ + public Optional<String> leafRequestId() { + return leafRequestId; } public List<String> requestedDnsSans() { @@ -78,7 +90,8 @@ public class EndpointCertificateMetadata { this.certName, version, this.lastRequested, - this.requestId, + this.rootRequestId, + this.leafRequestId, this.requestedDnsSans, this.issuer, this.expiry, @@ -91,7 +104,8 @@ public class EndpointCertificateMetadata { this.certName, this.version, lastRequested, - this.requestId, + this.rootRequestId, + this.leafRequestId, this.requestedDnsSans, this.issuer, this.expiry, @@ -104,20 +118,36 @@ public class EndpointCertificateMetadata { this.certName, this.version, this.lastRequested, - this.requestId, + this.rootRequestId, + this.leafRequestId, this.requestedDnsSans, this.issuer, this.expiry, Optional.of(lastRefreshed)); } - public EndpointCertificateMetadata withRequestId(String requestId) { + public EndpointCertificateMetadata withRootRequestId(String rootRequestId) { + return new EndpointCertificateMetadata( + this.keyName, + this.certName, + this.version, + this.lastRequested, + rootRequestId, + this.leafRequestId, + this.requestedDnsSans, + this.issuer, + this.expiry, + lastRefreshed); + } + + public EndpointCertificateMetadata withLeafRequestId(Optional<String> leafRequestId) { return new EndpointCertificateMetadata( this.keyName, this.certName, this.version, this.lastRequested, - requestId, + this.rootRequestId, + leafRequestId, this.requestedDnsSans, this.issuer, this.expiry, @@ -127,16 +157,17 @@ public class EndpointCertificateMetadata { @Override public String toString() { return "EndpointCertificateMetadata{" + - "keyName='" + keyName + '\'' + - ", certName='" + certName + '\'' + - ", version=" + version + - ", lastRequested=" + lastRequested + - ", requestId=" + requestId + - ", requestedDnsSans=" + requestedDnsSans + - ", issuer=" + issuer + - ", expiry=" + expiry + - ", lastRefreshed=" + lastRefreshed + - '}'; + "keyName='" + keyName + '\'' + + ", certName='" + certName + '\'' + + ", version=" + version + + ", lastRequested=" + lastRequested + + ", rootRequestId=" + rootRequestId + + ", leafRequestId=" + leafRequestId + + ", requestedDnsSans=" + requestedDnsSans + + ", issuer=" + issuer + + ", expiry=" + expiry + + ", lastRefreshed=" + lastRefreshed + + '}'; } @Override @@ -146,18 +177,19 @@ public class EndpointCertificateMetadata { EndpointCertificateMetadata that = (EndpointCertificateMetadata) o; return version == that.version && lastRequested == that.lastRequested && - keyName.equals(that.keyName) && - certName.equals(that.certName) && - requestId.equals(that.requestId) && - requestedDnsSans.equals(that.requestedDnsSans) && - issuer.equals(that.issuer) && - expiry.equals(that.expiry) && - lastRefreshed.equals(that.lastRefreshed); + keyName.equals(that.keyName) && + certName.equals(that.certName) && + rootRequestId.equals(that.rootRequestId) && + leafRequestId.equals(that.leafRequestId) && + requestedDnsSans.equals(that.requestedDnsSans) && + issuer.equals(that.issuer) && + expiry.equals(that.expiry) && + lastRefreshed.equals(that.lastRefreshed); } @Override public int hashCode() { - return Objects.hash(keyName, certName, version, lastRequested, requestId, requestedDnsSans, issuer, expiry, lastRefreshed); + return Objects.hash(keyName, certName, version, lastRequested, rootRequestId, leafRequestId, requestedDnsSans, issuer, expiry, lastRefreshed); } } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/EndpointCertificateMock.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/EndpointCertificateMock.java index 3e484a5669b..19dc852da21 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/EndpointCertificateMock.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/EndpointCertificateMock.java @@ -33,8 +33,10 @@ public class EndpointCertificateMock implements EndpointCertificateProvider { long epochSecond = Instant.now().getEpochSecond(); long inAnHour = epochSecond + 3600; String requestId = UUID.randomUUID().toString(); - EndpointCertificateMetadata metadata = new EndpointCertificateMetadata(endpointCertificatePrefix + "-key", endpointCertificatePrefix + "-cert", 0, 0, - requestId, dnsNames, "mockCa", Optional.of(inAnHour), Optional.of(epochSecond)); + int version = currentMetadata.map(c -> currentMetadata.get().version()+1).orElse(0); + EndpointCertificateMetadata metadata = new EndpointCertificateMetadata(endpointCertificatePrefix + "-key", endpointCertificatePrefix + "-cert", version, 0, + currentMetadata.map(EndpointCertificateMetadata::rootRequestId).orElse(requestId), Optional.of(requestId), dnsNames, "mockCa", Optional.of(inAnHour), Optional.of(epochSecond)); + currentMetadata.ifPresent(c -> providerMetadata.remove(c.leafRequestId().orElseThrow())); providerMetadata.put(requestId, metadata); return metadata; } @@ -44,10 +46,10 @@ public class EndpointCertificateMock implements EndpointCertificateProvider { return providerMetadata.values().stream() .map(p -> new EndpointCertificateRequestMetadata( - p.requestId(), - "mock", - "mock", - "mock", + p.leafRequestId().orElse(p.rootRequestId()), + "requestor", + "ticketId", + "athenzDomain", p.requestedDnsSans().stream() .map(san -> new EndpointCertificateRequestMetadata.DnsNameStatus(san, "done")) .collect(Collectors.toUnmodifiableList()), @@ -67,4 +69,30 @@ public class EndpointCertificateMock implements EndpointCertificateProvider { providerMetadata.remove(requestId); } + @Override + public EndpointCertificateDetails certificateDetails(String requestId) { + var metadata = providerMetadata.get(requestId); + + if(metadata==null) throw new RuntimeException("Unknown certificate request"); + + return new EndpointCertificateDetails(requestId, + "requestor", + "ok", + "ticket_id", + "athenz_domain", + metadata.requestedDnsSans().stream().map(name -> new EndpointCertificateRequestMetadata.DnsNameStatus(name, "done")).collect(Collectors.toList()), + "duration_sec", + "expiry", + metadata.keyName(), + metadata.keyName(), + "0", + metadata.certName(), + metadata.certName(), + "0", + "2021-09-28T00:14:31.946562037Z", + true, + "public_key_algo", + "issuer", + "serial"); + } } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/EndpointCertificateProvider.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/EndpointCertificateProvider.java index f4e41a25b79..26db25bd848 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/EndpointCertificateProvider.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/certificates/EndpointCertificateProvider.java @@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.controller.api.integration.certificates; import com.yahoo.config.provision.ApplicationId; +import java.io.IOException; import java.util.List; import java.util.Optional; @@ -18,4 +19,6 @@ public interface EndpointCertificateProvider { List<EndpointCertificateRequestMetadata> listCertificates(); void deleteCertificate(ApplicationId applicationId, String requestId); + + EndpointCertificateDetails certificateDetails(String requestId); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificates.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificates.java index e3091b704e4..5e19b014083 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificates.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificates.java @@ -81,7 +81,7 @@ public class EndpointCertificates { if (!currentCertificateMetadata.get().requestedDnsSans().containsAll(requiredSansForZone)) { var reprovisionedCertificateMetadata = provisionEndpointCertificate(deployment, currentCertificateMetadata, deploymentSpec) - .withRequestId(currentCertificateMetadata.get().requestId()); // We're required to keep the original request ID + .withRootRequestId(currentCertificateMetadata.get().rootRequestId()); // We're required to keep the original request ID curator.writeEndpointCertificateMetadata(instance.id(), reprovisionedCertificateMetadata); // Verification is unlikely to succeed in this case, as certificate must be available first - controller will retry certificateValidator.validate(reprovisionedCertificateMetadata, instance.id().serializedForm(), zone, requiredSansForZone); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java index 3f12cce9aae..f36f5be7778 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java @@ -33,6 +33,7 @@ import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.function.Function; +import java.util.function.Predicate; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -139,10 +140,10 @@ public class DeploymentStatus { return dependents.contains(current); } - /** Whether any job is failing on anything older than version, with errors other than lack of capacity in a test zone.. */ - public boolean hasFailures(ApplicationVersion version) { + /** Whether any job is failing on versions selected by the given filter, with errors other than lack of capacity in a test zone.. */ + public boolean hasFailures(Predicate<ApplicationVersion> versionFilter) { return ! allJobs.failingHard() - .matching(job -> job.lastTriggered().get().versions().targetApplication().compareTo(version) < 0) + .matching(job -> versionFilter.test(job.lastTriggered().get().versions().targetApplication())) .isEmpty(); } @@ -210,7 +211,7 @@ public class DeploymentStatus { Versions versions = Versions.from(change, application, firstProductionJobWithDeployment.flatMap(this::deploymentFor), systemVersion); if (step.completedAt(change, firstProductionJobWithDeployment).isEmpty()) - jobs.merge(job, List.of(new Job(versions, step.readyAt(change), change)), DeploymentStatus::union); + jobs.merge(job, List.of(new Job(job.type(), versions, step.readyAt(change), change)), DeploymentStatus::union); }); return Collections.unmodifiableMap(jobs); } @@ -303,20 +304,29 @@ public class DeploymentStatus { private Map<JobId, List<Job>> productionJobs(InstanceName instance, Change change, boolean assumeUpgradesSucceed) { Map<JobId, List<Job>> jobs = new LinkedHashMap<>(); jobSteps.forEach((job, step) -> { - // When computing eager test jobs for outstanding changes, assume current upgrade completes successfully. - Optional<Deployment> deployment = deploymentFor(job) - .map(existing -> assumeUpgradesSucceed ? withChange(existing, change.withoutApplication()) : existing); + // When computing eager test jobs for outstanding changes, assume current change completes successfully. + Optional<Deployment> deployment = deploymentFor(job); + Optional<Version> existingPlatform = deployment.map(Deployment::version); + Optional<ApplicationVersion> existingApplication = deployment.map(Deployment::applicationVersion); + if (assumeUpgradesSucceed) { + Change currentChange = application.require(instance).change(); + Versions target = Versions.from(currentChange, application, deployment, systemVersion); + existingPlatform = Optional.of(target.targetPlatform()); + existingApplication = Optional.of(target.targetApplication()); + } if (job.application().instance().equals(instance) && job.type().isProduction()) { - List<Job> toRun = new ArrayList<>(); List<Change> changes = changes(job, step, change); if (changes.isEmpty()) return; for (Change partial : changes) { - toRun.add(new Job(Versions.from(partial, application, deployment, systemVersion), - step.readyAt(partial, Optional.of(job)), - partial)); + Job jobToRun = new Job(job.type(), + Versions.from(partial, application, existingPlatform, existingApplication, systemVersion), + step.readyAt(partial, Optional.of(job)), + partial); + toRun.add(jobToRun); // Assume first partial change is applied before the second. - deployment = deployment.map(existing -> withChange(existing, partial)); + existingPlatform = Optional.of(jobToRun.versions.targetPlatform()); + existingApplication = Optional.of(jobToRun.versions.targetApplication()); } jobs.put(job, toRun); } @@ -324,17 +334,6 @@ public class DeploymentStatus { return jobs; } - private static Deployment withChange(Deployment deployment, Change change) { - return new Deployment(deployment.zone(), - change.application().orElse(deployment.applicationVersion()), - change.platform().orElse(deployment.version()), - deployment.at(), - deployment.metrics(), - deployment.activity(), - deployment.quota(), - deployment.cost()); - } - /** Changes to deploy with the given job, possibly split in two steps. */ private List<Change> changes(JobId job, StepStatus step, Change change) { // Signal strict completion criterion by depending on job itself. @@ -432,7 +431,8 @@ public class DeploymentStatus { declaredTest(job.application(), testType).ifPresent(testJob -> { for (Job productionJob : versionsList) if (allJobs.successOn(productionJob.versions()).get(testJob).isEmpty()) - testJobs.merge(testJob, List.of(new Job(productionJob.versions(), + testJobs.merge(testJob, List.of(new Job(testJob.type(), + productionJob.versions(), jobSteps().get(testJob).readyAt(productionJob.change), productionJob.change)), DeploymentStatus::union); @@ -448,7 +448,8 @@ public class DeploymentStatus { && testJobs.get(test).stream().anyMatch(testJob -> testJob.versions().equals(productionJob.versions())))) { JobId testJob = firstDeclaredOrElseImplicitTest(testType); testJobs.merge(testJob, - List.of(new Job(productionJob.versions(), + List.of(new Job(testJob.type(), + productionJob.versions(), jobSteps.get(testJob).readyAt(productionJob.change), productionJob.change)), DeploymentStatus::union); @@ -872,8 +873,8 @@ public class DeploymentStatus { private final Optional<Instant> readyAt; private final Change change; - public Job(Versions versions, Optional<Instant> readyAt, Change change) { - this.versions = versions; + public Job(JobType type, Versions versions, Optional<Instant> readyAt, Change change) { + this.versions = type == systemTest ? versions.withoutSources() : versions; this.readyAt = readyAt; this.change = change; } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java index bb8180e9f14..20fa539c820 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.hosted.controller.deployment; import com.yahoo.config.application.api.DeploymentInstanceSpec; +import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.InstanceName; import com.yahoo.text.Text; @@ -30,6 +31,7 @@ import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.OptionalLong; +import java.util.function.Predicate; import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; @@ -197,7 +199,7 @@ public class DeploymentTrigger { DeploymentStatus status = jobs.deploymentStatus(application); Versions versions = Versions.from(instance.change(), application, status.deploymentFor(job), controller.readSystemVersion()); - DeploymentStatus.Job toTrigger = new DeploymentStatus.Job(versions, Optional.of(controller.clock().instant()), instance.change()); + DeploymentStatus.Job toTrigger = new DeploymentStatus.Job(job.type(), versions, Optional.of(controller.clock().instant()), instance.change()); Map<JobId, List<DeploymentStatus.Job>> jobs = status.testJobs(Map.of(job, List.of(toTrigger))); if (jobs.isEmpty() || ! requireTests) jobs = Map.of(job, List.of(toTrigger)); @@ -388,9 +390,13 @@ public class DeploymentTrigger { private boolean acceptNewApplicationVersion(DeploymentStatus status, InstanceName instance, ApplicationVersion version) { if (status.application().deploymentSpec().instance(instance).isEmpty()) return false; // Unknown instance. boolean isChangingRevision = status.application().require(instance).change().application().isPresent(); - switch (status.application().deploymentSpec().requireInstance(instance).revisionChange()) { + DeploymentInstanceSpec spec = status.application().deploymentSpec().requireInstance(instance); + Predicate<ApplicationVersion> versionFilter = spec.revisionTarget() == DeploymentSpec.RevisionTarget.next + ? failing -> status.application().require(instance).change().application().get().compareTo(failing) == 0 + : failing -> version.compareTo(failing) > 0; + switch (spec.revisionChange()) { case whenClear: return ! isChangingRevision; - case whenFailing: return ! isChangingRevision || status.hasFailures(version); + case whenFailing: return ! isChangingRevision || status.hasFailures(versionFilter); case always: return true; default: throw new IllegalStateException("Unknown revision upgrade policy"); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java index 87268f88363..b7bfade98a6 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java @@ -22,6 +22,9 @@ import com.yahoo.security.SignatureAlgorithm; import com.yahoo.security.X509CertificateBuilder; import com.yahoo.security.X509CertificateUtils; import com.yahoo.text.Text; +import com.yahoo.vespa.flags.FetchVector; +import com.yahoo.vespa.flags.FlagSource; +import com.yahoo.vespa.flags.Flags; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.Instance; @@ -674,15 +677,12 @@ public class InternalStepRunner implements StepRunner { controller.jobController().updateTestReport(id); return Optional.of(error); case NO_TESTS: - if (isSetup) { - return Optional.of(running); - } TesterCloud.Suite suite = TesterCloud.Suite.of(id.type(), isSetup); logger.log(INFO, "No tests were found in the test package, for test suite '" + suite + "'"); logger.log(INFO, "The test package must either contain basic HTTP tests under 'tests/<suite-name>/', " + "or a Java test bundle under 'components/' with at least one test with the annotation " + "for this suite. See docs.vespa.ai/en/testing.html for details."); - return Optional.of(running); // Let no tests pass until all apps meet this requirement. + return Optional.of(allowNoTests(id.application()) ? running : testFailure); case SUCCESS: logger.log("Tests completed successfully."); controller.jobController().updateTestReport(id); @@ -692,6 +692,12 @@ public class InternalStepRunner implements StepRunner { } } + private boolean allowNoTests(ApplicationId appId) { + return Flags.ALLOW_NO_TESTS.bindTo(controller.flagSource()) + .with(FetchVector.Dimension.TENANT_ID, appId.tenant().value()) + .value(); + } + private Optional<RunStatus> copyVespaLogs(RunId id, DualLogger logger) { if (deployment(id.application(), id.type()).isPresent()) try { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Versions.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Versions.java index 1e183d44377..0ea18d9cfa2 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Versions.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Versions.java @@ -37,6 +37,11 @@ public class Versions { this.sourceApplication = requireNonNull(sourceApplication); } + /** A copy of this, without source versions. */ + public Versions withoutSources() { + return new Versions(targetPlatform, targetApplication, Optional.empty(), Optional.empty()); + } + /** Target platform version for this */ public Version targetPlatform() { return targetPlatform; @@ -98,36 +103,36 @@ public class Versions { } /** Create versions using given change and application */ + public static Versions from(Change change, Application application, Optional<Version> existingPlatform, + Optional<ApplicationVersion> existingApplication, Version defaultPlatformVersion) { + return new Versions(targetPlatform(application, change, existingPlatform, defaultPlatformVersion), + targetApplication(application, change, existingApplication), + existingPlatform, + existingApplication); + } + + /** Create versions using given change and application */ public static Versions from(Change change, Application application, Optional<Deployment> deployment, Version defaultPlatformVersion) { - return new Versions(targetPlatform(application, change, deployment, defaultPlatformVersion), - targetApplication(application, change, deployment), + return new Versions(targetPlatform(application, change, deployment.map(Deployment::version), defaultPlatformVersion), + targetApplication(application, change, deployment.map(Deployment::applicationVersion)), deployment.map(Deployment::version), deployment.map(Deployment::applicationVersion)); } - public static Versions from(Change change, Deployment deployment) { - return new Versions(change.platform().filter(version -> change.isPinned() || deployment.version().isBefore(version)) - .orElse(deployment.version()), - change.application().filter(version -> deployment.applicationVersion().compareTo(version) < 0) - .orElse(deployment.applicationVersion()), - Optional.of(deployment.version()), - Optional.of(deployment.applicationVersion())); - } - - private static Version targetPlatform(Application application, Change change, Optional<Deployment> deployment, + private static Version targetPlatform(Application application, Change change, Optional<Version> existing, Version defaultVersion) { if (change.isPinned() && change.platform().isPresent()) return change.platform().get(); - return max(change.platform(), deployment.map(Deployment::version)) + return max(change.platform(), existing) .orElseGet(() -> application.oldestDeployedPlatform().orElse(defaultVersion)); } private static ApplicationVersion targetApplication(Application application, Change change, - Optional<Deployment> deployment) { + Optional<ApplicationVersion> existing) { return change.application() - .or(() -> deployment.map(Deployment::applicationVersion)) + .or(() -> existing) .orElseGet(() -> defaultApplicationVersion(application)); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainer.java index cc00572e760..3f54958fc45 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainer.java @@ -11,6 +11,7 @@ import com.yahoo.vespa.flags.BooleanFlag; import com.yahoo.vespa.flags.Flags; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.Instance; +import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateDetails; import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateMetadata; import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateProvider; import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateRequestMetadata; @@ -28,7 +29,6 @@ import java.util.List; import java.util.Map; import java.util.Optional; import java.util.OptionalInt; -import java.util.Set; import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; @@ -135,7 +135,7 @@ public class EndpointCertificateMaintainer extends ControllerMaintainer { if (Optional.of(storedMetaData).equals(curator.readEndpointCertificateMetadata(applicationId))) { log.log(Level.INFO, "Cert for app " + applicationId.serializedForm() + " has not been requested in a month and app has no deployments, deleting from provider and ZK"); - endpointCertificateProvider.deleteCertificate(applicationId, storedMetaData.requestId()); + endpointCertificateProvider.deleteCertificate(applicationId, storedMetaData.rootRequestId()); curator.deleteEndpointCertificateMetadata(applicationId); } } @@ -149,30 +149,59 @@ public class EndpointCertificateMaintainer extends ControllerMaintainer { private boolean hasNoDeployments(ApplicationId applicationId) { return controller().applications().getInstance(applicationId) - .map(Instance::deployments) - .orElseGet(Map::of) - .isEmpty(); + .map(Instance::deployments) + .orElseGet(Map::of) + .isEmpty(); } private void deleteOrReportUnmanagedCertificates() { List<EndpointCertificateRequestMetadata> endpointCertificateMetadata = endpointCertificateProvider.listCertificates(); - Set<String> managedRequestIds = curator.readAllEndpointCertificateMetadata().values().stream().map(EndpointCertificateMetadata::requestId).collect(Collectors.toSet()); + Map<ApplicationId, EndpointCertificateMetadata> storedEndpointCertificateMetadata = curator.readAllEndpointCertificateMetadata(); + + List<String> leafRequestIds = storedEndpointCertificateMetadata.values().stream().flatMap(m -> m.leafRequestId().stream()).collect(Collectors.toList()); + List<String> rootRequestIds = storedEndpointCertificateMetadata.values().stream().map(EndpointCertificateMetadata::rootRequestId).collect(Collectors.toList()); for (var providerCertificateMetadata : endpointCertificateMetadata) { - if (!managedRequestIds.contains(providerCertificateMetadata.requestId())) { - if (deleteUnmaintainedCertificates.value()) { - // The certificate is not known - however it could be in the process of being requested by us or another controller. - // So we only delete if it was requested more than 7 days ago. - if (Instant.parse(providerCertificateMetadata.createTime()).isBefore(Instant.now().minus(7, ChronoUnit.DAYS))) { - log.log(Level.INFO, String.format("Deleting unmaintained certificate with request_id %s and SANs %s", + if (!rootRequestIds.contains(providerCertificateMetadata.requestId()) && !leafRequestIds.contains(providerCertificateMetadata.requestId())) { + + // It could just be a refresh we're not aware of yet. See if it matches the cert/keyname of any known cert + EndpointCertificateDetails unknownCertDetails = endpointCertificateProvider.certificateDetails(providerCertificateMetadata.requestId()); + boolean matchFound = false; + for (Map.Entry<ApplicationId, EndpointCertificateMetadata> storedAppEntry : storedEndpointCertificateMetadata.entrySet()) { + ApplicationId storedApp = storedAppEntry.getKey(); + EndpointCertificateMetadata storedAppMetadata = storedAppEntry.getValue(); + if (storedAppMetadata.certName().equals(unknownCertDetails.cert_key_keyname())) { + matchFound = true; + try (Lock lock = lock(storedApp)) { + if (Optional.of(storedAppMetadata).equals(curator.readEndpointCertificateMetadata(storedApp))) { + if (deleteUnmaintainedCertificates.value()) { + log.log(Level.INFO, "Cert for app " + storedApp.serializedForm() + + " has a new leafRequestId " + unknownCertDetails.request_id() + ", updating in ZK"); + curator.writeEndpointCertificateMetadata(storedApp, storedAppMetadata.withLeafRequestId(Optional.of(unknownCertDetails.request_id()))); + } else { + log.log(Level.INFO, "Cert for app " + storedApp.serializedForm() + + " has a new leafRequestId " + unknownCertDetails.request_id()); + } + } + break; + } + } + } + if (!matchFound) { + if (deleteUnmaintainedCertificates.value()) { + // The certificate is not known - however it could be in the process of being requested by us or another controller. + // So we only delete if it was requested more than 7 days ago. + if (Instant.parse(providerCertificateMetadata.createTime()).isBefore(Instant.now().minus(7, ChronoUnit.DAYS))) { + log.log(Level.INFO, String.format("Deleting unmaintained certificate with request_id %s and SANs %s", + providerCertificateMetadata.requestId(), + providerCertificateMetadata.dnsNames().stream().map(d -> d.dnsName).collect(Collectors.joining(", ")))); + endpointCertificateProvider.deleteCertificate(ApplicationId.fromSerializedForm("applicationid:is:unknown"), providerCertificateMetadata.requestId()); + } + } else { + log.log(Level.INFO, () -> String.format("Found unmaintained certificate with request_id %s and SANs %s", providerCertificateMetadata.requestId(), providerCertificateMetadata.dnsNames().stream().map(d -> d.dnsName).collect(Collectors.joining(", ")))); - endpointCertificateProvider.deleteCertificate(ApplicationId.fromSerializedForm("applicationid:is:unknown"), providerCertificateMetadata.requestId()); } - } else { - log.log(Level.FINE, () -> String.format("Found unmaintained certificate with request_id %s and SANs %s", - providerCertificateMetadata.requestId(), - providerCertificateMetadata.dnsNames().stream().map(d -> d.dnsName).collect(Collectors.joining(", ")))); } } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/EndpointCertificateMetadataSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/EndpointCertificateMetadataSerializer.java index 310b3637b84..e1181f1cede 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/EndpointCertificateMetadataSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/EndpointCertificateMetadataSerializer.java @@ -33,7 +33,8 @@ public class EndpointCertificateMetadataSerializer { private final static String certNameField = "certName"; private final static String versionField = "version"; private final static String lastRequestedField = "lastRequested"; - private final static String requestIdField = "requestId"; + private final static String rootRequestIdField = "requestId"; + private final static String leafRequestIdField = "leafRequestId"; private final static String requestedDnsSansField = "requestedDnsSans"; private final static String issuerField = "issuer"; private final static String expiryField = "expiry"; @@ -46,7 +47,8 @@ public class EndpointCertificateMetadataSerializer { object.setString(certNameField, metadata.certName()); object.setLong(versionField, metadata.version()); object.setLong(lastRequestedField, metadata.lastRequested()); - object.setString(requestIdField, metadata.requestId()); + object.setString(rootRequestIdField, metadata.rootRequestId()); + metadata.leafRequestId().ifPresent(leafRequestId -> object.setString(leafRequestIdField, leafRequestId)); var cursor = object.setArray(requestedDnsSansField); metadata.requestedDnsSans().forEach(cursor::addString); object.setString(issuerField, metadata.issuer()); @@ -65,7 +67,8 @@ public class EndpointCertificateMetadataSerializer { inspector.field(certNameField).asString(), Math.toIntExact(inspector.field(versionField).asLong()), inspector.field(lastRequestedField).asLong(), - inspector.field(requestIdField).asString(), + inspector.field(rootRequestIdField).asString(), + SlimeUtils.optionalString(inspector.field(leafRequestIdField)), IntStream.range(0, inspector.field(requestedDnsSansField).entries()) .mapToObj(i -> inspector.field(requestedDnsSansField).entry(i).asString()).collect(Collectors.toList()), inspector.field(issuerField).asString(), diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleFilter.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleFilter.java index da048e6b569..766a51e3e8d 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleFilter.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/AthenzRoleFilter.java @@ -152,7 +152,8 @@ public class AthenzRoleFilter extends JsonSecurityRequestFilterBase { && application.isPresent()) { ZoneId z = zone.get(); futures.add(executor.submit(() -> { - if (canDeployToManualZones(identity, ((AthenzTenant) tenant.get()).domain(), application.get(), z)) + if (tenant.get().type() == Tenant.Type.athenz + && canDeployToManualZones(identity, ((AthenzTenant) tenant.get()).domain(), application.get(), z)) roleMemberships.add(Role.hostedDeveloper(tenant.get().name())); })); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificatesTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificatesTest.java index 0a93c936b41..61bcb119d00 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificatesTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificatesTest.java @@ -40,6 +40,7 @@ import java.util.Optional; import java.util.Set; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertTrue; /** @@ -160,7 +161,7 @@ public class EndpointCertificatesTest { @Test public void reuses_stored_certificate_metadata() { - mockCuratorDb.writeEndpointCertificateMetadata(testInstance.id(), new EndpointCertificateMetadata(testKeyName, testCertName, 7, 0, "request_id", + mockCuratorDb.writeEndpointCertificateMetadata(testInstance.id(), new EndpointCertificateMetadata(testKeyName, testCertName, 7, 0, "request_id", Optional.of("leaf-request-uuid"), List.of("vt2ktgkqme5zlnp4tj4ttyor7fj3v7q5o.vespa.oath.cloud", "default.default.global.vespa.oath.cloud", "*.default.default.global.vespa.oath.cloud", @@ -178,7 +179,7 @@ public class EndpointCertificatesTest { @Test public void reprovisions_certificate_when_necessary() { - mockCuratorDb.writeEndpointCertificateMetadata(testInstance.id(), new EndpointCertificateMetadata(testKeyName, testCertName, -1, 0, "uuid", List.of(), "issuer", Optional.empty(), Optional.empty())); + mockCuratorDb.writeEndpointCertificateMetadata(testInstance.id(), new EndpointCertificateMetadata(testKeyName, testCertName, -1, 0, "root-request-uuid", Optional.of("leaf-request-uuid"), List.of(), "issuer", Optional.empty(), Optional.empty())); secretStore.setSecret("vespa.tls.default.default.default-key", KeyUtils.toPem(testKeyPair.getPrivate()), 0); secretStore.setSecret("vespa.tls.default.default.default-cert", X509CertificateUtils.toPem(testCertificate) + X509CertificateUtils.toPem(testCertificate), 0); Optional<EndpointCertificateMetadata> endpointCertificateMetadata = endpointCertificates.getMetadata(testInstance, testZone, DeploymentSpec.empty); @@ -191,7 +192,7 @@ public class EndpointCertificatesTest { public void reprovisions_certificate_with_added_sans_when_deploying_to_new_zone() { ZoneId testZone = tester.zoneRegistry().zones().routingMethod(RoutingMethod.exclusive).in(Environment.prod).zones().stream().skip(1).findFirst().orElseThrow().getId(); - mockCuratorDb.writeEndpointCertificateMetadata(testInstance.id(), new EndpointCertificateMetadata(testKeyName, testCertName, -1, 0, "original-request-uuid", expectedSans, "mockCa", Optional.empty(), Optional.empty())); + mockCuratorDb.writeEndpointCertificateMetadata(testInstance.id(), new EndpointCertificateMetadata(testKeyName, testCertName, -1, 0, "original-request-uuid", Optional.of("leaf-request-uuid"), expectedSans, "mockCa", Optional.empty(), Optional.empty())); secretStore.setSecret("vespa.tls.default.default.default-key", KeyUtils.toPem(testKeyPair.getPrivate()), -1); secretStore.setSecret("vespa.tls.default.default.default-cert", X509CertificateUtils.toPem(testCertificate) + X509CertificateUtils.toPem(testCertificate), -1); @@ -202,7 +203,8 @@ public class EndpointCertificatesTest { assertTrue(endpointCertificateMetadata.isPresent()); assertEquals(0, endpointCertificateMetadata.get().version()); assertEquals(endpointCertificateMetadata, mockCuratorDb.readEndpointCertificateMetadata(testInstance.id())); - assertEquals("original-request-uuid", endpointCertificateMetadata.get().requestId()); + assertEquals("original-request-uuid", endpointCertificateMetadata.get().rootRequestId()); + assertNotEquals(Optional.of("leaf-request-uuid"), endpointCertificateMetadata.get().leafRequestId()); assertEquals(Set.copyOf(expectedCombinedSans), Set.copyOf(endpointCertificateMetadata.get().requestedDnsSans())); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java index bfedd325ae7..805d727d355 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java @@ -251,9 +251,6 @@ public class DeploymentTriggerTest { .build(); DeploymentContext app = tester.newDeploymentContext() .submit(appPackage); - Optional<ApplicationVersion> revision0 = app.lastSubmission(); - - app.submit(appPackage); Optional<ApplicationVersion> revision1 = app.lastSubmission(); app.submit(appPackage); @@ -262,17 +259,47 @@ public class DeploymentTriggerTest { app.submit(appPackage); Optional<ApplicationVersion> revision3 = app.lastSubmission(); - assertEquals(revision0, app.instance().change().application()); - assertEquals(revision1, app.deploymentStatus().outstandingChange(InstanceName.defaultName()).application()); + app.submit(appPackage); + Optional<ApplicationVersion> revision4 = app.lastSubmission(); + + app.submit(appPackage); + Optional<ApplicationVersion> revision5 = app.lastSubmission(); - tester.deploymentTrigger().forceChange(app.instanceId(), Change.of(revision1.get())); + // 5 revisions submitted; the first is rolling out, and the others are queued. + tester.outstandingChangeDeployer().run(); assertEquals(revision1, app.instance().change().application()); assertEquals(revision2, app.deploymentStatus().outstandingChange(InstanceName.defaultName()).application()); - app.deploy(); + // The second revision is set as the target by user interaction. + tester.deploymentTrigger().forceChange(app.instanceId(), Change.of(revision2.get())); tester.outstandingChangeDeployer().run(); assertEquals(revision2, app.instance().change().application()); assertEquals(revision3, app.deploymentStatus().outstandingChange(InstanceName.defaultName()).application()); + + // The second revision deploys completely, and the third starts rolling out. + app.runJob(systemTest).runJob(stagingTest) + .runJob(productionUsEast3); + tester.outstandingChangeDeployer().run(); + tester.outstandingChangeDeployer().run(); + assertEquals(revision3, app.instance().change().application()); + assertEquals(revision4, app.deploymentStatus().outstandingChange(InstanceName.defaultName()).application()); + + // The third revision fails, and the fourth is chosen to replace it. + app.triggerJobs().timeOutConvergence(systemTest); + tester.outstandingChangeDeployer().run(); + tester.outstandingChangeDeployer().run(); + assertEquals(revision4, app.instance().change().application()); + assertEquals(revision5, app.deploymentStatus().outstandingChange(InstanceName.defaultName()).application()); + + // Tests for outstanding change are relevant when current revision completes. + app.runJob(systemTest).runJob(systemTest) + .jobAborted(stagingTest).runJob(stagingTest).runJob(stagingTest) + .runJob(productionUsEast3); + tester.outstandingChangeDeployer().run(); + tester.outstandingChangeDeployer().run(); + assertEquals(revision5, app.instance().change().application()); + assertEquals(Change.empty(), app.deploymentStatus().outstandingChange(InstanceName.defaultName())); + app.runJob(productionUsEast3); } @Test @@ -1810,7 +1837,8 @@ public class DeploymentTriggerTest { // System and staging tests both require unknown versions, and are broken. tester.controller().applications().deploymentTrigger().forceTrigger(app.instanceId(), productionCdUsEast1, "user", false); app.runJob(productionCdUsEast1) - .abortJob(systemTest) + .triggerJobs() + .jobAborted(systemTest) .jobAborted(stagingTest) .runJob(systemTest) // Run test for aws zone again. .runJob(stagingTest) // Run test for aws zone again. diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainerTest.java index 24ac473a7b7..5be39b4a733 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/EndpointCertificateMaintainerTest.java @@ -11,7 +11,6 @@ import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder; import com.yahoo.vespa.hosted.controller.deployment.DeploymentContext; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester; import com.yahoo.vespa.hosted.controller.integration.SecretStoreMock; -import com.yahoo.yolean.concurrent.Sleeper; import org.junit.Test; import java.time.Duration; @@ -22,6 +21,7 @@ import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobTy import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType.stagingTest; import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType.systemTest; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertTrue; /** @@ -32,7 +32,7 @@ public class EndpointCertificateMaintainerTest { private final ControllerTester tester = new ControllerTester(); private final SecretStoreMock secretStore = (SecretStoreMock) tester.controller().secretStore(); private final EndpointCertificateMaintainer maintainer = new EndpointCertificateMaintainer(tester.controller(), Duration.ofHours(1)); - private final EndpointCertificateMetadata exampleMetadata = new EndpointCertificateMetadata("keyName", "certName", 0, 0, "uuid", List.of(), "issuer", Optional.empty(), Optional.empty()); + private final EndpointCertificateMetadata exampleMetadata = new EndpointCertificateMetadata("keyName", "certName", 0, 0, "root-request-uuid", Optional.of("leaf-request-uuid"), List.of(), "issuer", Optional.empty(), Optional.empty()); { ((InMemoryFlagSource) tester.controller().flagSource()).withBooleanFlag(Flags.DELETE_UNMAINTAINED_CERTIFICATES.id(), true); } @@ -81,15 +81,13 @@ public class EndpointCertificateMaintainerTest { deploymentContext.submit(applicationPackage).runJob(systemTest).runJob(stagingTest).runJob(productionUsWest1); - - tester.curator().writeEndpointCertificateMetadata(appId, exampleMetadata); - assertEquals(1.0, maintainer.maintain(), 0.0000001); - assertTrue(tester.curator().readEndpointCertificateMetadata(appId).isPresent()); // cert should not be deleted, the app is deployed! + var metadata = tester.curator().readEndpointCertificateMetadata(appId).orElseThrow(); + tester.controller().serviceRegistry().endpointCertificateProvider().certificateDetails(metadata.rootRequestId()); // cert should not be deleted, the app is deployed! } @Test - public void refreshed_certificate_is_deployed_after_four_days() { + public void refreshed_certificate_is_discovered_and_after_four_days_deployed() { var appId = ApplicationId.from("tenant", "application", "default"); DeploymentTester deploymentTester = new DeploymentTester(tester); @@ -99,27 +97,31 @@ public class EndpointCertificateMaintainerTest { .build(); DeploymentContext deploymentContext = deploymentTester.newDeploymentContext("tenant", "application", "default"); - deploymentContext.submit(applicationPackage).runJob(systemTest).runJob(stagingTest).runJob(productionUsWest1); + var originalMetadata = tester.curator().readEndpointCertificateMetadata(appId).orElseThrow(); - tester.curator().writeEndpointCertificateMetadata(appId, exampleMetadata); - + // cert should not be deleted, the app is deployed! assertEquals(1.0, maintainer.maintain(), 0.0000001); - assertTrue(tester.curator().readEndpointCertificateMetadata(appId).isPresent()); // cert should not be deleted, the app is deployed! + assertEquals(tester.curator().readEndpointCertificateMetadata(appId), Optional.of(originalMetadata)); + tester.controller().serviceRegistry().endpointCertificateProvider().certificateDetails(originalMetadata.rootRequestId()); + // This simulates a cert refresh performed 3 days later tester.clock().advance(Duration.ofDays(3)); + secretStore.setSecret(originalMetadata.keyName(), "foo", 1); + secretStore.setSecret(originalMetadata.certName(), "bar", 1); + tester.controller().serviceRegistry().endpointCertificateProvider().requestCaSignedCertificate(appId, originalMetadata.requestedDnsSans(), Optional.of(originalMetadata)); - secretStore.setSecret(exampleMetadata.keyName(), "foo", 1); - secretStore.setSecret(exampleMetadata.certName(), "bar", 1); - - maintainer.maintain(); + // We should now pick up the new key and cert version + uuid, but not force trigger deployment yet + assertEquals(1.0, maintainer.maintain(), 0.0000001); + deploymentContext.assertNotRunning(productionUsWest1); + var updatedMetadata = tester.curator().readEndpointCertificateMetadata(appId).orElseThrow(); + assertNotEquals(originalMetadata.leafRequestId().orElseThrow(), updatedMetadata.leafRequestId().orElseThrow()); + assertEquals(updatedMetadata.version(), originalMetadata.version()+1); + // after another 4 days, we should force trigger deployment if it hasn't already happened tester.clock().advance(Duration.ofDays(4)); - deploymentContext.assertNotRunning(productionUsWest1); - - maintainer.maintain(); - + assertEquals(1.0, maintainer.maintain(), 0.0000001); deploymentContext.assertRunning(productionUsWest1); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/EndpointCertificateMetadataSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/EndpointCertificateMetadataSerializerTest.java index f2171032e98..383f5038416 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/EndpointCertificateMetadataSerializerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/EndpointCertificateMetadataSerializerTest.java @@ -11,39 +11,39 @@ import static org.junit.Assert.*; public class EndpointCertificateMetadataSerializerTest { - private final EndpointCertificateMetadata sampleWithExpiryAndLastRefreshed = - new EndpointCertificateMetadata("keyName", "certName", 1, 0, "requestId", List.of("SAN1", "SAN2"), "issuer", java.util.Optional.of(1628000000L), Optional.of(1612000000L)); + private final EndpointCertificateMetadata sampleWithOptionalFieldsSet = + new EndpointCertificateMetadata("keyName", "certName", 1, 0, "rootRequestId", Optional.of("leafRequestId"), List.of("SAN1", "SAN2"), "issuer", java.util.Optional.of(1628000000L), Optional.of(1612000000L)); - private final EndpointCertificateMetadata sampleWithoutExpiry = - new EndpointCertificateMetadata("keyName", "certName", 1, 0, "requestId", List.of("SAN1", "SAN2"), "issuer", Optional.empty(), Optional.empty()); + private final EndpointCertificateMetadata sampleWithoutOptionalFieldsSet = + new EndpointCertificateMetadata("keyName", "certName", 1, 0, "rootRequestId", Optional.empty(), List.of("SAN1", "SAN2"), "issuer", Optional.empty(), Optional.empty()); @Test - public void serializeWithExpiryAndLastRefreshed() { + public void serialize_with_optional_fields() { assertEquals( - "{\"keyName\":\"keyName\",\"certName\":\"certName\",\"version\":1,\"lastRequested\":0,\"requestId\":\"requestId\",\"requestedDnsSans\":[\"SAN1\",\"SAN2\"],\"issuer\":\"issuer\",\"expiry\":1628000000,\"lastRefreshed\":1612000000}", - EndpointCertificateMetadataSerializer.toSlime(sampleWithExpiryAndLastRefreshed).toString()); + "{\"keyName\":\"keyName\",\"certName\":\"certName\",\"version\":1,\"lastRequested\":0,\"requestId\":\"rootRequestId\",\"leafRequestId\":\"leafRequestId\",\"requestedDnsSans\":[\"SAN1\",\"SAN2\"],\"issuer\":\"issuer\",\"expiry\":1628000000,\"lastRefreshed\":1612000000}", + EndpointCertificateMetadataSerializer.toSlime(sampleWithOptionalFieldsSet).toString()); } @Test - public void serializeWithoutExpiryAndLastRefreshed() { + public void serialize_without_optional_fields() { assertEquals( - "{\"keyName\":\"keyName\",\"certName\":\"certName\",\"version\":1,\"lastRequested\":0,\"requestId\":\"requestId\",\"requestedDnsSans\":[\"SAN1\",\"SAN2\"],\"issuer\":\"issuer\"}", - EndpointCertificateMetadataSerializer.toSlime(sampleWithoutExpiry).toString()); + "{\"keyName\":\"keyName\",\"certName\":\"certName\",\"version\":1,\"lastRequested\":0,\"requestId\":\"rootRequestId\",\"requestedDnsSans\":[\"SAN1\",\"SAN2\"],\"issuer\":\"issuer\"}", + EndpointCertificateMetadataSerializer.toSlime(sampleWithoutOptionalFieldsSet).toString()); } @Test - public void deserializeFromJsonWithExpiryAndLastRefreshed() { + public void deserialize_from_json_with_optional_fields() { assertEquals( - sampleWithExpiryAndLastRefreshed, + sampleWithOptionalFieldsSet, EndpointCertificateMetadataSerializer.fromJsonString( - "{\"keyName\":\"keyName\",\"certName\":\"certName\",\"version\":1,\"lastRequested\":0,\"requestId\":\"requestId\",\"requestedDnsSans\":[\"SAN1\",\"SAN2\"],\"issuer\":\"issuer\",\"expiry\":1628000000,\"lastRefreshed\":1612000000}")); + "{\"keyName\":\"keyName\",\"certName\":\"certName\",\"version\":1,\"lastRequested\":0,\"requestId\":\"rootRequestId\",\"leafRequestId\":\"leafRequestId\",\"requestedDnsSans\":[\"SAN1\",\"SAN2\"],\"issuer\":\"issuer\",\"expiry\":1628000000,\"lastRefreshed\":1612000000}")); } @Test - public void deserializeFromJsonWithoutExpiryAndLastRefreshed() { + public void deserialize_from_json_without_optional_fields() { assertEquals( - sampleWithoutExpiry, + sampleWithoutOptionalFieldsSet, EndpointCertificateMetadataSerializer.fromJsonString( - "{\"keyName\":\"keyName\",\"certName\":\"certName\",\"version\":1,\"lastRequested\":0,\"requestId\":\"requestId\",\"requestedDnsSans\":[\"SAN1\",\"SAN2\"],\"issuer\":\"issuer\"}")); + "{\"keyName\":\"keyName\",\"certName\":\"certName\",\"version\":1,\"lastRequested\":0,\"requestId\":\"rootRequestId\",\"requestedDnsSans\":[\"SAN1\",\"SAN2\"],\"issuer\":\"issuer\"}")); } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview-2.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview-2.json index b05be36a24a..ed82039d923 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview-2.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview-2.json @@ -144,13 +144,6 @@ "compileVersion": "6.1.0", "sourceUrl": "repository1/tree/commit1", "commit": "commit1" - }, - "sourcePlatform": "6.1.0", - "sourceApplication": { - "build": 2, - "compileVersion": "6.1.0", - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" } }, "steps": [ @@ -209,13 +202,6 @@ "compileVersion": "6.1.0", "sourceUrl": "repository1/tree/commit1", "commit": "commit1" - }, - "sourcePlatform": "6.1.0", - "sourceApplication": { - "build": 1, - "compileVersion": "6.1.0", - "sourceUrl": "repository1/tree/commit1", - "commit": "commit1" } }, "steps": [ diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/responses/root.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/responses/root.json index c5286d4a04b..211aa57d8ed 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/responses/root.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/responses/root.json @@ -194,7 +194,7 @@ { "name": "system-test", "coolingDownUntil": "(ignore)", - "pending": "platform" + "pending": "application" }, { "name": "staging-test", 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 8cf5e93f93d..881b62d1e04 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -383,6 +383,13 @@ public class Flags { "Takes effect immediately", ZONE_ID); + public static final UnboundBooleanFlag ALLOW_NO_TESTS = defineFeatureFlag( + "allow-no-tests", false, + List.of("jonmv"), "2022-02-28", "2022-06-25", + "Whether test jobs without any tests run are acceptable", + "Takes effect immediately", + TENANT_ID); + /** WARNING: public for testing: All flags should be defined in {@link Flags}. */ public static UnboundBooleanFlag defineFeatureFlag(String flagId, boolean defaultValue, List<String> owners, String createdAt, String expiresAt, String description, diff --git a/hosted-api/src/main/java/ai/vespa/hosted/api/ControllerHttpClient.java b/hosted-api/src/main/java/ai/vespa/hosted/api/ControllerHttpClient.java index 11a4d096312..3189a2c8e92 100644 --- a/hosted-api/src/main/java/ai/vespa/hosted/api/ControllerHttpClient.java +++ b/hosted-api/src/main/java/ai/vespa/hosted/api/ControllerHttpClient.java @@ -117,7 +117,8 @@ public abstract class ControllerHttpClient { POST, new MultiPartStreamer().addJson("submitOptions", metaToJson(submission)) .addFile("applicationZip", submission.applicationZip()) - .addFile("applicationTestZip", submission.applicationTestZip())))); + .addFile("applicationTestZip", submission.applicationTestZip())), + 1)); } /** Sends the given deployment to the given application in the given zone, or throws if this fails. */ @@ -125,7 +126,8 @@ public abstract class ControllerHttpClient { return toDeploymentResult(send(request(HttpRequest.newBuilder(deploymentJobPath(id, zone)) .timeout(Duration.ofMinutes(20)), POST, - toDataStream(deployment)))); + toDataStream(deployment)), + 1)); } /** Deactivates the deployment of the given application in the given zone. */ @@ -333,8 +335,16 @@ public abstract class ControllerHttpClient { /** Returns a response with a 2XX status code, with up to 10 attempts, or throws. */ private HttpResponse<byte[]> send(HttpRequest request) { + return send(request, 10); + } + + /** Returns a response with a 2XX status code, after the specified number of attempts, or throws. */ + private HttpResponse<byte[]> send(HttpRequest request, int attempts) { + if (attempts < 1) + throw new IllegalStateException("Programming error, attempts must be at least 1"); + UncheckedIOException thrown = null; - for (int attempt = 1; attempt <= 10; attempt++) { + for (int attempt = 1; attempt <= attempts; attempt++) { try { HttpResponse<byte[]> response = client.send(request, ofByteArray()); if (response.statusCode() / 100 == 2) diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java index b99a3bb84d7..a524243e2fb 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java @@ -201,7 +201,8 @@ public class RealNodeRepository implements NodeRepository { nodeResources.diskGb, nodeResources.bandwidthGbps, diskSpeedFromString(nodeResources.diskSpeed), - storageTypeFromString(nodeResources.storageType)); + storageTypeFromString(nodeResources.storageType), + architectureFromString(nodeResources.architecture)); } private static NodeResources.DiskSpeed diskSpeedFromString(String diskSpeed) { @@ -224,6 +225,16 @@ public class RealNodeRepository implements NodeRepository { } } + private static NodeResources.Architecture architectureFromString(String architecture) { + if (architecture == null) return NodeResources.Architecture.getDefault(); + switch (architecture) { + case "arm64": return NodeResources.Architecture.arm64; + case "x86_64": return NodeResources.Architecture.x86_64; + case "any": return NodeResources.Architecture.any; + default: throw new IllegalArgumentException("Unknown architecture '" + architecture + "'"); + } + } + private static String toString(NodeResources.DiskSpeed diskSpeed) { switch (diskSpeed) { case fast : return "fast"; @@ -242,6 +253,15 @@ public class RealNodeRepository implements NodeRepository { } } + private static String toString(NodeResources.Architecture architecture) { + switch (architecture) { + case arm64 : return "arm64"; + case x86_64 : return "x86_64"; + case any : return "any"; + default: throw new IllegalArgumentException("Unknown architecture '" + architecture.name() + "'"); + } + } + private static NodeRepositoryNode nodeRepositoryNodeFromAddNode(AddNode addNode) { NodeRepositoryNode node = new NodeRepositoryNode(); node.id = addNode.id; @@ -260,6 +280,7 @@ public class RealNodeRepository implements NodeRepository { node.resources.bandwidthGbps = resources.bandwidthGbps(); node.resources.diskSpeed = toString(resources.diskSpeed()); node.resources.storageType = toString(resources.storageType()); + node.resources.architecture = toString(resources.architecture()); }); node.type = addNode.nodeType.name(); node.ipAddresses = addNode.ipAddresses; diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/bindings/NodeRepositoryNode.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/bindings/NodeRepositoryNode.java index f99fb3d8b76..726ca391c8f 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/bindings/NodeRepositoryNode.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/bindings/NodeRepositoryNode.java @@ -191,6 +191,8 @@ public class NodeRepositoryNode { public String diskSpeed; @JsonProperty public String storageType; + @JsonProperty + public String architecture; @Override public String toString() { @@ -201,6 +203,7 @@ public class NodeRepositoryNode { ", bandwidthGbps=" + bandwidthGbps + ", diskSpeed='" + diskSpeed + '\'' + ", storageType='" + storageType + '\'' + + ", architecture='" + architecture + '\'' + '}'; } } diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepositoryTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepositoryTest.java index 3256b16a6c5..3f07a8f5c90 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepositoryTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepositoryTest.java @@ -178,6 +178,7 @@ public class RealNodeRepositoryTest { assertEquals("default", hostSpec.flavor()); assertEquals(123, hostSpec.diskGb(), 0); assertEquals(NodeType.confighost, hostSpec.type()); + assertEquals(NodeResources.Architecture.x86_64, hostSpec.resources().architecture()); NodeSpec nodeSpec = nodeRepositoryApi.getOptionalNode("host123-1.domain.tld").orElseThrow(); assertEquals(nodeResources, nodeSpec.resources()); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeResourcesSerializer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeResourcesSerializer.java index 1c3d3f5c489..f7be2b510c7 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeResourcesSerializer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeResourcesSerializer.java @@ -18,6 +18,7 @@ public class NodeResourcesSerializer { private static final String bandwidthKey = "bandwidth"; private static final String diskSpeedKey = "diskSpeed"; private static final String storageTypeKey = "storageType"; + private static final String architectureKey = "architecture"; static void toSlime(NodeResources resources, Cursor resourcesObject) { if (resources.isUnspecified()) return; @@ -27,6 +28,7 @@ public class NodeResourcesSerializer { resourcesObject.setDouble(bandwidthKey, resources.bandwidthGbps()); resourcesObject.setString(diskSpeedKey, diskSpeedToString(resources.diskSpeed())); resourcesObject.setString(storageTypeKey, storageTypeToString(resources.storageType())); + resourcesObject.setString(architectureKey, architectureToString(resources.architecture())); } static NodeResources resourcesFromSlime(Inspector resources) { @@ -36,7 +38,8 @@ public class NodeResourcesSerializer { resources.field(diskKey).asDouble(), resources.field(bandwidthKey).asDouble(), diskSpeedFromSlime(resources.field(diskSpeedKey)), - storageTypeFromSlime(resources.field(storageTypeKey))); + storageTypeFromSlime(resources.field(storageTypeKey)), + architectureFromSlime(resources.field(architectureKey))); } static Optional<NodeResources> optionalResourcesFromSlime(Inspector resources) { @@ -62,7 +65,6 @@ public class NodeResourcesSerializer { } private static NodeResources.StorageType storageTypeFromSlime(Inspector storageType) { - if ( ! storageType.valid()) return NodeResources.StorageType.getDefault(); // TODO: Remove this line after December 2019 switch (storageType.asString()) { case "remote" : return NodeResources.StorageType.remote; case "local" : return NodeResources.StorageType.local; @@ -80,4 +82,23 @@ public class NodeResourcesSerializer { } } + private static NodeResources.Architecture architectureFromSlime(Inspector architecture) { + if ( ! architecture.valid()) return NodeResources.Architecture.getDefault(); // TODO: Remove this line after March 2022 + switch (architecture.asString()) { + case "arm64" : return NodeResources.Architecture.arm64; + case "x86_64" : return NodeResources.Architecture.x86_64; + case "any" : return NodeResources.Architecture.any; + default: throw new IllegalStateException("Illegal architecture value '" + architecture.asString() + "'"); + } + } + + private static String architectureToString(NodeResources.Architecture architecture) { + switch (architecture) { + case arm64 : return "arm64"; + case x86_64 : return "x86_64"; + case any : return "any"; + default: throw new IllegalStateException("Illegal architecture value '" + architecture + "'"); + } + } + } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/FlavorConfigBuilder.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/FlavorConfigBuilder.java index 2994b21f56b..4567d596c35 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/FlavorConfigBuilder.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/FlavorConfigBuilder.java @@ -82,6 +82,8 @@ public class FlavorConfigBuilder { flavorConfigBuilder.addFlavor(flavorName, 48, 128, 1000, 10, Flavor.Type.BARE_METAL); else if (flavorName.equals("devhost")) flavorConfigBuilder.addFlavor(flavorName, 4., 80., 100, 10, Flavor.Type.BARE_METAL); + else if (flavorName.equals("arm64")) + flavorConfigBuilder.addFlavor(flavorName,2., 30., 20., 3, Flavor.Type.BARE_METAL, Architecture.arm64); else flavorConfigBuilder.addFlavor(flavorName, 1., 30., 20., 3, Flavor.Type.BARE_METAL); } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializerTest.java index 3de3f4139d1..cc121ba8104 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializerTest.java @@ -44,6 +44,9 @@ import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; +import static com.yahoo.config.provision.NodeResources.Architecture; +import static com.yahoo.config.provision.NodeResources.DiskSpeed; +import static com.yahoo.config.provision.NodeResources.StorageType; import static java.time.temporal.ChronoUnit.MILLIS; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -55,7 +58,7 @@ import static org.junit.Assert.assertTrue; */ public class NodeSerializerTest { - private final NodeFlavors nodeFlavors = FlavorConfigBuilder.createDummies("default", "large", "ugccloud-container"); + private final NodeFlavors nodeFlavors = FlavorConfigBuilder.createDummies("default", "large", "ugccloud-container", "arm64"); private final NodeSerializer nodeSerializer = new NodeSerializer(nodeFlavors, 1000); private final ManualClock clock = new ManualClock(); @@ -74,7 +77,8 @@ public class NodeSerializerTest { @Test public void reserved_node_serialization() { Node node = createNode(); - NodeResources requestedResources = new NodeResources(1.2, 3.4, 5.6, 7.8, NodeResources.DiskSpeed.any); + NodeResources requestedResources = new NodeResources(1.2, 3.4, 5.6, 7.8, + DiskSpeed.any, StorageType.any, Architecture.arm64); clock.advance(Duration.ofMinutes(3)); assertEquals(0, node.history().events().size()); @@ -87,7 +91,7 @@ public class NodeSerializerTest { assertEquals(1, node.history().events().size()); node = node.withRestart(new Generation(1, 2)); node = node.withReboot(new Generation(3, 4)); - node = node.with(FlavorConfigBuilder.createDummies("large").getFlavorOrThrow("large"), Agent.system, clock.instant()); + node = node.with(FlavorConfigBuilder.createDummies("arm64").getFlavorOrThrow("arm64"), Agent.system, clock.instant()); node = node.with(node.status().withVespaVersion(Version.fromString("1.2.3"))); node = node.with(node.status().withIncreasedFailCount().withIncreasedFailCount()); node = node.with(NodeType.tenant); @@ -100,7 +104,8 @@ public class NodeSerializerTest { assertEquals(2, copy.allocation().get().restartGeneration().current()); assertEquals(3, copy.status().reboot().wanted()); assertEquals(4, copy.status().reboot().current()); - assertEquals("large", copy.flavor().name()); + assertEquals("arm64", copy.flavor().name()); + assertEquals(Architecture.arm64.name(), copy.resources().architecture().name()); assertEquals("1.2.3", copy.status().vespaVersion().get().toString()); assertEquals(2, copy.status().failCount()); assertEquals(node.allocation().get().owner(), copy.allocation().get().owner()); diff --git a/searchcore/src/vespa/searchcore/proton/server/proton.cpp b/searchcore/src/vespa/searchcore/proton/server/proton.cpp index fd174e784d0..974c2522565 100644 --- a/searchcore/src/vespa/searchcore/proton/server/proton.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/proton.cpp @@ -808,7 +808,7 @@ Proton::updateMetrics(const metrics::MetricLockGuard &) metrics.shared.update(_shared_service->shared().getStats()); metrics.warmup.update(_shared_service->warmup().getStats()); if (_shared_service->field_writer()) { - metrics.warmup.update(_shared_service->field_writer()->getStats()); + metrics.field_writer.update(_shared_service->field_writer()->getStats()); } } } diff --git a/searchlib/abi-spec.json b/searchlib/abi-spec.json index 5d7e281df87..3213b1bb2b9 100644 --- a/searchlib/abi-spec.json +++ b/searchlib/abi-spec.json @@ -1609,13 +1609,14 @@ "methods": [ "public void <init>()", "public void <init>(java.util.Collection)", + "public void <init>(java.util.Collection, java.util.Optional)", "public void <init>(java.util.Map)", "public void <init>(java.util.Collection, java.util.Map)", "public void <init>(java.util.Collection, java.util.Map, com.yahoo.tensor.evaluation.TypeContext)", - "public java.util.Optional typeContext()", "public void <init>(java.util.Map, java.util.Map, java.util.Map)", "public void <init>(java.util.Map, java.util.Map, java.util.Optional, java.util.Map)", "public void <init>(com.google.common.collect.ImmutableMap, java.util.Map, java.util.Map)", + "public java.util.Optional typeContext()", "public void addFunctionSerialization(java.lang.String, java.lang.String)", "public void addArgumentTypeSerialization(java.lang.String, java.lang.String, com.yahoo.tensor.TensorType)", "public void addFunctionTypeSerialization(java.lang.String, com.yahoo.tensor.TensorType)", diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/RankingExpression.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/RankingExpression.java index 5a73d89cf2c..780d738eb2b 100755 --- a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/RankingExpression.java +++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/RankingExpression.java @@ -265,6 +265,7 @@ public class RankingExpression implements Serializable { } @Deprecated + @SuppressWarnings("removal") public Map<String, String> getRankProperties(List<ExpressionFunction> functions) { return getRankProperties(new SerializationContext(functions)); } diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/SerializationContext.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/SerializationContext.java index 1f3203f2e35..d0a8e812091 100644 --- a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/SerializationContext.java +++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/SerializationContext.java @@ -32,17 +32,24 @@ public class SerializationContext extends FunctionReferenceContext { this(Collections.emptyList()); } - /** Create a context for a single serialization task */ + /** @deprecated Use {@link #SerializationContext(Collection, Optional) instead}*/ + @Deprecated(forRemoval = true, since = "7") public SerializationContext(Collection<ExpressionFunction> functions) { this(functions, Collections.emptyMap(), Optional.empty(), new LinkedHashMap<>()); } - /** Create a context for a single serialization task */ + public SerializationContext(Collection<ExpressionFunction> functions, Optional<TypeContext<Reference>> typeContext) { + this(functions, Collections.emptyMap(), typeContext, new LinkedHashMap<>()); + } + + /** @deprecated Use {@link #SerializationContext(Map, Map, Optional, Map) instead}*/ + @Deprecated(forRemoval = true, since = "7") public SerializationContext(Map<String, ExpressionFunction> functions) { this(functions.values()); } - /** Create a context for a single serialization task */ + /** @deprecated Use {@link #SerializationContext(Collection, Map, TypeContext) instead}*/ + @Deprecated(forRemoval = true, since = "7") public SerializationContext(Collection<ExpressionFunction> functions, Map<String, String> bindings) { this(functions, bindings, Optional.empty(), new LinkedHashMap<>()); } @@ -63,35 +70,19 @@ public class SerializationContext extends FunctionReferenceContext { * is <b>transferred</b> to this and will be modified in it */ private SerializationContext(Collection<ExpressionFunction> functions, Map<String, String> bindings, - Optional<TypeContext<Reference>> typeContext, - Map<String, String> serializedFunctions) { + Optional<TypeContext<Reference>> typeContext, + Map<String, String> serializedFunctions) { this(toMap(functions), bindings, typeContext, serializedFunctions); } - /** Returns the type context of this, if it is able to resolve types. */ - public Optional<TypeContext<Reference>> typeContext() { return typeContext; } - - private static Map<String, ExpressionFunction> toMap(Collection<ExpressionFunction> list) { - Map<String,ExpressionFunction> mapBuilder = new HashMap<>(); - for (ExpressionFunction function : list) - mapBuilder.put(function.getName(), function); - return Map.copyOf(mapBuilder); - } - - /** - * Create a context for a single serialization task - * - * @param functions the functions of this - * @param bindings the arguments of this - * @param serializedFunctions a cache of serializedFunctions - the ownership of this map - * is <b>transferred</b> to this and will be modified in it - */ + /** @deprecated Use {@link #SerializationContext(Map, Map, Optional, Map) instead}*/ + @Deprecated(forRemoval = true, since = "7") public SerializationContext(Map<String, ExpressionFunction> functions, Map<String, String> bindings, Map<String, String> serializedFunctions) { this(functions, bindings, Optional.empty(), serializedFunctions); } - public SerializationContext(Map<String,ExpressionFunction> functions, Map<String, String> bindings, + public SerializationContext(Map<String, ExpressionFunction> functions, Map<String, String> bindings, Optional<TypeContext<Reference>> typeContext, Map<String, String> serializedFunctions) { super(functions, bindings); @@ -99,13 +90,23 @@ public class SerializationContext extends FunctionReferenceContext { this.serializedFunctions = serializedFunctions; } - /** @deprecated Use {@link #SerializationContext(Map, Map, Map) instead}*/ + /** @deprecated Use {@link #SerializationContext(Map, Map, Optional, Map) instead}*/ @Deprecated(forRemoval = true, since = "7") public SerializationContext(ImmutableMap<String,ExpressionFunction> functions, Map<String, String> bindings, Map<String, String> serializedFunctions) { this((Map<String, ExpressionFunction>)functions, bindings, serializedFunctions); } + /** Returns the type context of this, if it is able to resolve types. */ + public Optional<TypeContext<Reference>> typeContext() { return typeContext; } + + private static Map<String, ExpressionFunction> toMap(Collection<ExpressionFunction> list) { + Map<String,ExpressionFunction> mapBuilder = new HashMap<>(); + for (ExpressionFunction function : list) + mapBuilder.put(function.getName(), function); + return Map.copyOf(mapBuilder); + } + /** Adds the serialization of a function */ public void addFunctionSerialization(String name, String expressionString) { serializedFunctions.put(name, expressionString); @@ -124,13 +125,13 @@ public class SerializationContext extends FunctionReferenceContext { @Override public SerializationContext withBindings(Map<String, String> bindings) { - return new SerializationContext(getFunctions(), bindings, this.serializedFunctions); + return new SerializationContext(getFunctions(), bindings, typeContext, this.serializedFunctions); } /** Returns a fresh context without bindings */ @Override public SerializationContext withoutBindings() { - return new SerializationContext(getFunctions(), null, this.serializedFunctions); + return new SerializationContext(getFunctions(), null, typeContext, this.serializedFunctions); } public Map<String, String> serializedFunctions() { return serializedFunctions; } diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/TensorFunctionNode.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/TensorFunctionNode.java index 52d54c9163e..ce5832027b7 100644 --- a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/TensorFunctionNode.java +++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/TensorFunctionNode.java @@ -360,17 +360,22 @@ public class TensorFunctionNode extends CompositeNode { /** Returns a new context with the bindings replaced by the given bindings */ @Override public ExpressionToStringContext withBindings(Map<String, String> bindings) { - SerializationContext serializationContext = new SerializationContext(getFunctions(), bindings, serializedFunctions()); + SerializationContext serializationContext = new SerializationContext(getFunctions(), bindings, typeContext(), serializedFunctions()); return new ExpressionToStringContext(serializationContext, wrappedToStringContext, path, parent); } /** Returns a fresh context without bindings */ @Override public SerializationContext withoutBindings() { - SerializationContext serializationContext = new SerializationContext(getFunctions(), null, serializedFunctions()); + SerializationContext serializationContext = new SerializationContext(getFunctions(), null, typeContext(), serializedFunctions()); return new ExpressionToStringContext(serializationContext, null, path, parent); } + @Override + public String toString() { + return "TensorFunctionNode.ExpressionToStringContext with wrapped serialization context: " + wrappedSerializationContext; + } + } /** Turns an EvaluationContext into a Context */ diff --git a/searchlib/src/test/java/com/yahoo/searchlib/rankingexpression/RankingExpressionTestCase.java b/searchlib/src/test/java/com/yahoo/searchlib/rankingexpression/RankingExpressionTestCase.java index 24b7b98b74e..1ab9ee11252 100755 --- a/searchlib/src/test/java/com/yahoo/searchlib/rankingexpression/RankingExpressionTestCase.java +++ b/searchlib/src/test/java/com/yahoo/searchlib/rankingexpression/RankingExpressionTestCase.java @@ -26,6 +26,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.concurrent.Callable; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -97,7 +98,7 @@ public class RankingExpressionTestCase { RankingExpression exp = new RankingExpression("foo"); try { - exp.getRankProperties(new SerializationContext(functions)); + exp.getRankProperties(new SerializationContext(functions, Optional.empty())); } catch (RuntimeException e) { assertEquals("Cycle in ranking expression function: [foo[]]", e.getMessage()); } @@ -111,7 +112,7 @@ public class RankingExpressionTestCase { RankingExpression exp = new RankingExpression("foo"); try { - exp.getRankProperties(new SerializationContext(functions)); + exp.getRankProperties(new SerializationContext(functions, Optional.empty())); } catch (RuntimeException e) { assertEquals("Cycle in ranking expression function: [foo[], bar[]]", e.getMessage()); } @@ -391,14 +392,16 @@ public class RankingExpressionTestCase { List<ExpressionFunction> functions) { assertSerialization(expectedSerialization, expressionString, functions, false); } - private void assertSerialization(List<String> expectedSerialization, String expressionString, + + private void assertSerialization(List<String> expectedSerialization, String expressionString, List<ExpressionFunction> functions, boolean print) { try { if (print) System.out.println("Parsing expression '" + expressionString + "':"); RankingExpression expression = new RankingExpression(expressionString); - Map<String, String> rankProperties = expression.getRankProperties(new SerializationContext(functions)); + Map<String, String> rankProperties = expression.getRankProperties(new SerializationContext(functions, + Optional.empty())); if (print) { for (String key : rankProperties.keySet()) System.out.println(key + ": " + rankProperties.get(key)); diff --git a/storage/src/tests/storageserver/communicationmanagertest.cpp b/storage/src/tests/storageserver/communicationmanagertest.cpp index f8462d528c7..05dc1c642d0 100644 --- a/storage/src/tests/storageserver/communicationmanagertest.cpp +++ b/storage/src/tests/storageserver/communicationmanagertest.cpp @@ -45,6 +45,24 @@ struct CommunicationManagerTest : Test { } }; +namespace { + +void +wait_for_slobrok_visibility(const CommunicationManager& mgr, + const api::StorageMessageAddress& addr) +{ + const auto deadline = vespalib::steady_clock::now() + 60s; + do { + if (mgr.address_visible_in_slobrok(addr)) { + return; + } + std::this_thread::sleep_for(10ms); + } while (vespalib::steady_clock::now() < deadline); + FAIL() << "Timed out waiting for address " << addr.toString() << " to be visible in Slobrok"; +} + +} + TEST_F(CommunicationManagerTest, simple) { mbus::Slobrok slobrok; vdstestlib::DirConfig distConfig(getStandardConfig(false)); @@ -70,12 +88,19 @@ TEST_F(CommunicationManagerTest, simple) { distributor.open(); storage.open(); - std::this_thread::sleep_for(1s); + auto stor_addr = api::StorageMessageAddress::create(&_Storage, lib::NodeType::STORAGE, 1); + auto distr_addr = api::StorageMessageAddress::create(&_Storage, lib::NodeType::DISTRIBUTOR, 1); + // It is undefined when the logical nodes will be visible in each others Slobrok + // mirrors, so explicitly wait until mutual visibility is ensured. Failure to do this + // might cause the below message to be immediately bounced due to failing to map the + // storage address to an actual RPC endpoint. + ASSERT_NO_FATAL_FAILURE(wait_for_slobrok_visibility(distributor, stor_addr)); + ASSERT_NO_FATAL_FAILURE(wait_for_slobrok_visibility(storage, distr_addr)); // Send a message through from distributor to storage auto cmd = std::make_shared<api::GetCommand>( makeDocumentBucket(document::BucketId(0)), document::DocumentId("id:ns:mytype::mydoc"), document::AllFields::NAME); - cmd->setAddress(api::StorageMessageAddress::create(&_Storage, lib::NodeType::STORAGE, 1)); + cmd->setAddress(stor_addr); distributorLink->sendUp(cmd); storageLink->waitForMessages(1, MESSAGE_WAIT_TIME_SEC); ASSERT_GT(storageLink->getNumCommands(), 0); diff --git a/storage/src/vespa/storage/storageserver/communicationmanager.cpp b/storage/src/vespa/storage/storageserver/communicationmanager.cpp index 237dc76d783..975e9361072 100644 --- a/storage/src/vespa/storage/storageserver/communicationmanager.cpp +++ b/storage/src/vespa/storage/storageserver/communicationmanager.cpp @@ -6,6 +6,7 @@ #include <vespa/messagebus/emptyreply.h> #include <vespa/messagebus/network/rpcnetworkparams.h> #include <vespa/messagebus/rpcmessagebus.h> +#include <vespa/slobrok/sbmirror.h> #include <vespa/storage/common/bucket_resolver.h> #include <vespa/storage/common/nodestateupdater.h> #include <vespa/storage/config/config-stor-server.h> @@ -806,4 +807,11 @@ void CommunicationManager::updateBucketSpacesConfig(const BucketspacesConfig& co _docApiConverter.setBucketResolver(ConfigurableBucketResolver::from_config(config)); } +bool +CommunicationManager::address_visible_in_slobrok(const api::StorageMessageAddress& addr) const noexcept +{ + assert(_storage_api_rpc_service); + return _storage_api_rpc_service->address_visible_in_slobrok_uncached(addr); +} + } // storage diff --git a/storage/src/vespa/storage/storageserver/communicationmanager.h b/storage/src/vespa/storage/storageserver/communicationmanager.h index 80117b32030..31c6fa00f0e 100644 --- a/storage/src/vespa/storage/storageserver/communicationmanager.h +++ b/storage/src/vespa/storage/storageserver/communicationmanager.h @@ -164,6 +164,10 @@ public: void updateBucketSpacesConfig(const BucketspacesConfig&); const CommunicationManagerMetrics& metrics() const noexcept { return _metrics; } + + // Intended primarily for unit tests that fire up multiple nodes and must wait until all + // nodes are cross-visible in Slobrok before progressing. + [[nodiscard]] bool address_visible_in_slobrok(const api::StorageMessageAddress& addr) const noexcept; }; } // storage diff --git a/storage/src/vespa/storage/storageserver/rpc/shared_rpc_resources.cpp b/storage/src/vespa/storage/storageserver/rpc/shared_rpc_resources.cpp index e1a2dc6b03c..7ad59ee574c 100644 --- a/storage/src/vespa/storage/storageserver/rpc/shared_rpc_resources.cpp +++ b/storage/src/vespa/storage/storageserver/rpc/shared_rpc_resources.cpp @@ -110,7 +110,12 @@ void SharedRpcResources::shutdown() { assert(!_shutdown); if (listen_port() > 0) { _slobrok_register->unregisterName(_handle); + // Give slobrok some time to dispatch unregister RPC + while (_slobrok_register->busy()) { + std::this_thread::sleep_for(10ms); + } } + _slobrok_register.reset(); // Implicitly kill any pending slobrok tasks prior to shutting down transport layer _transport->ShutDown(true); // FIXME need to reset to break weak_ptrs? But ShutDown should already sync pending resolves...! _shutdown = true; diff --git a/storage/src/vespa/storage/storageserver/rpc/storage_api_rpc_service.cpp b/storage/src/vespa/storage/storageserver/rpc/storage_api_rpc_service.cpp index 06323199341..78a9956c334 100644 --- a/storage/src/vespa/storage/storageserver/rpc/storage_api_rpc_service.cpp +++ b/storage/src/vespa/storage/storageserver/rpc/storage_api_rpc_service.cpp @@ -394,6 +394,15 @@ bool StorageApiRpcService::target_supports_direct_rpc( return _direct_rpc_supported.load(std::memory_order_relaxed); } +bool +StorageApiRpcService::address_visible_in_slobrok_uncached( + const api::StorageMessageAddress& addr) const noexcept +{ + auto sb_id = CachingRpcTargetResolver::address_to_slobrok_id(addr); + auto specs = _rpc_resources.slobrok_mirror().lookup(sb_id); + return !specs.empty(); +} + /* * Major TODOs: diff --git a/storage/src/vespa/storage/storageserver/rpc/storage_api_rpc_service.h b/storage/src/vespa/storage/storageserver/rpc/storage_api_rpc_service.h index 94bf663837c..2526cf5434c 100644 --- a/storage/src/vespa/storage/storageserver/rpc/storage_api_rpc_service.h +++ b/storage/src/vespa/storage/storageserver/rpc/storage_api_rpc_service.h @@ -56,6 +56,8 @@ public: ~StorageApiRpcService() override; [[nodiscard]] bool target_supports_direct_rpc(const api::StorageMessageAddress& addr) const noexcept; + // Bypasses resolver cache and returns whether local Slobrok mirror has at least 1 spec for the given address. + [[nodiscard]] bool address_visible_in_slobrok_uncached(const api::StorageMessageAddress& addr) const noexcept; void RPC_rpc_v1_send(FRT_RPCRequest* req); void encode_rpc_v1_response(FRT_RPCRequest& request, api::StorageReply& reply); diff --git a/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/JunitRunner.java b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/JunitRunner.java index 12d50ca725c..a7c40ddfad2 100644 --- a/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/JunitRunner.java +++ b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/JunitRunner.java @@ -105,7 +105,7 @@ public class JunitRunner extends AbstractComponent implements TestRunner { logRecords.clear(); Optional<Bundle> testBundle = findTestBundle(); if (testBundle.isEmpty()) { - execution = CompletableFuture.completedFuture(TestReport.builder().build()); + execution = CompletableFuture.completedFuture(null); return execution; } @@ -231,7 +231,7 @@ public class JunitRunner extends AbstractComponent implements TestRunner { if (execution == null) return TestRunner.Status.NOT_STARTED; if ( ! execution.isDone()) return TestRunner.Status.RUNNING; try { - return execution.get().status(); + return execution.get() == null ? Status.NO_TESTS : execution.get().status(); } catch (InterruptedException|ExecutionException e) { logger.log(Level.WARNING, "Error while getting test report", e); return TestRunner.Status.ERROR; diff --git a/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/TestReport.java b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/TestReport.java index 4db5405029b..a9ac950e30a 100644 --- a/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/TestReport.java +++ b/vespa-osgi-testrunner/src/main/java/com/yahoo/vespa/testrunner/TestReport.java @@ -39,7 +39,7 @@ public class TestReport { } public TestRunner.Status status() { - return failedCount > 0 ? FAILURE : inconclusiveCount > 0 ? INCONCLUSIVE : (successCount + abortedCount + ignoredCount) > 0 ? SUCCESS : NO_TESTS; + return failedCount > 0 ? FAILURE : inconclusiveCount > 0 ? INCONCLUSIVE : successCount > 0 ? SUCCESS : NO_TESTS; } public static Builder builder(){ diff --git a/vespa-osgi-testrunner/src/test/java/com/yahoo/vespa/testrunner/AggregateTestRunnerTest.java b/vespa-osgi-testrunner/src/test/java/com/yahoo/vespa/testrunner/AggregateTestRunnerTest.java index 1af012f0fb2..f8e13ac5d6a 100644 --- a/vespa-osgi-testrunner/src/test/java/com/yahoo/vespa/testrunner/AggregateTestRunnerTest.java +++ b/vespa-osgi-testrunner/src/test/java/com/yahoo/vespa/testrunner/AggregateTestRunnerTest.java @@ -151,8 +151,8 @@ class AggregateTestRunnerTest { assertEquals(SUCCESS, TestReport.builder().withSuccessCount(1).build().status()); assertEquals(INCONCLUSIVE, TestReport.builder().withSuccessCount(1).withInconclusiveCount(1).build().status()); assertEquals(FAILURE, TestReport.builder().withSuccessCount(1).withFailedCount(1).build().status()); - assertEquals(SUCCESS, TestReport.builder().withAbortedCount(1).build().status()); - assertEquals(SUCCESS, TestReport.builder().withIgnoredCount(1).build().status()); + assertEquals(NO_TESTS, TestReport.builder().withAbortedCount(1).build().status()); + assertEquals(NO_TESTS, TestReport.builder().withIgnoredCount(1).build().status()); assertEquals(FAILURE, JunitRunner.createReportWithFailedInitialization(new RuntimeException("hello")).status()); } diff --git a/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/TestProfile.java b/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/TestProfile.java index 95a2b2723b8..09e2e218497 100644 --- a/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/TestProfile.java +++ b/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/TestProfile.java @@ -10,7 +10,7 @@ public enum TestProfile { SYSTEM_TEST("system, com.yahoo.vespa.tenant.systemtest.base.SystemTest", true), STAGING_SETUP_TEST("staging-setup", false), STAGING_TEST("staging, com.yahoo.vespa.tenant.systemtest.base.StagingTest", true), - PRODUCTION_TEST("production, com.yahoo.vespa.tenant.systemtest.base.ProductionTest", false); + PRODUCTION_TEST("production, com.yahoo.vespa.tenant.systemtest.base.ProductionTest", true); private final String group; private final boolean failIfNoTests; diff --git a/vespajlib/src/main/java/com/yahoo/tensor/functions/Slice.java b/vespajlib/src/main/java/com/yahoo/tensor/functions/Slice.java index da7581c39f9..e3464255fac 100644 --- a/vespajlib/src/main/java/com/yahoo/tensor/functions/Slice.java +++ b/vespajlib/src/main/java/com/yahoo/tensor/functions/Slice.java @@ -238,17 +238,21 @@ public class Slice<NAMETYPE extends Name> extends PrimitiveTensorFunction<NAMETY TensorType type = context.typeContext().isPresent() ? owner.argument.type(context.typeContext().get()) : null; if (type == null || type.dimensions().size() != 1) throw new IllegalArgumentException("The tensor dimension name being sliced by " + owner + - " cannot be uniquely resolved. Use the full form " + - "slice{myDimensionName: ..."); + " cannot be uniquely resolved. Use the full form: " + + "'slice{myDimensionName:" + valueToString(context) + "}'"); else dimensionName = Optional.of(type.dimensions().get(0).name()); } dimensionName.ifPresent(d -> b.append(d).append(":")); + b.append(valueToString(context)); + return b.toString(); + } + + private String valueToString(ToStringContext<NAMETYPE> context) { if (label != null) - b.append(label); + return label; else - b.append(index.toString(context)); - return b.toString(); + return index.toString(context); } } diff --git a/vespajlib/src/main/java/com/yahoo/tensor/functions/ToStringContext.java b/vespajlib/src/main/java/com/yahoo/tensor/functions/ToStringContext.java index 233779fcebe..72f0a267449 100644 --- a/vespajlib/src/main/java/com/yahoo/tensor/functions/ToStringContext.java +++ b/vespajlib/src/main/java/com/yahoo/tensor/functions/ToStringContext.java @@ -13,7 +13,7 @@ import java.util.Optional; */ public interface ToStringContext<NAMETYPE extends Name> { - static <NAMETYPE extends Name> ToStringContext<NAMETYPE> empty() { return new EmptyStringContext<NAMETYPE>(); } + static <NAMETYPE extends Name> ToStringContext<NAMETYPE> empty() { return new EmptyStringContext<>(); } /** Returns the name an identifier is bound to, or null if not bound in this context */ String getBinding(String name); |