diff options
113 files changed, 2296 insertions, 892 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/cert.go b/client/go/cmd/cert.go index 97147d1eec0..2a28589aabb 100644 --- a/client/go/cmd/cert.go +++ b/client/go/cmd/cert.go @@ -4,7 +4,9 @@ package cmd import ( + "errors" "fmt" + "io" "os" "path/filepath" @@ -13,14 +15,36 @@ import ( "github.com/vespa-engine/vespa/client/go/vespa" ) -var overwriteCertificate bool +var ( + noApplicationPackage bool + overwriteCertificate bool +) -const longDoc = `Create a new private key and self-signed certificate for Vespa Cloud deployment. +func init() { + certCmd.Flags().BoolVarP(&overwriteCertificate, "force", "f", false, "Force overwrite of existing certificate and private key") + certCmd.Flags().BoolVarP(&noApplicationPackage, "no-add", "N", false, "Do not add certificate to an application package") + certCmd.MarkPersistentFlagRequired(applicationFlag) + + deprecatedCertCmd.Flags().BoolVarP(&overwriteCertificate, "force", "f", false, "Force overwrite of existing certificate and private key") + deprecatedCertCmd.MarkPersistentFlagRequired(applicationFlag) + + certCmd.AddCommand(certAddCmd) + certAddCmd.Flags().BoolVarP(&overwriteCertificate, "force", "f", false, "Force overwrite of existing certificate") + certAddCmd.MarkPersistentFlagRequired(applicationFlag) +} + +var certCmd = &cobra.Command{ + Use: "cert", + Short: "Create a new private key and self-signed certificate for Vespa Cloud deployment", + Long: `Create a new private key and self-signed certificate for Vespa Cloud deployment. The private key and certificate will be stored in the Vespa CLI home directory (see 'vespa help config'). Other commands will then automatically load the certificate as necessary. +The certificate will be added to your application package specified as an +argument to this command (default '.'), unless the '--no-add' is set. + It's possible to override the private key and certificate used through environment variables. This can be useful in continuous integration systems. @@ -38,24 +62,9 @@ environment variables. This can be useful in continuous integration systems. Note that when overriding key pair through environment variables, that key pair will always be used for all applications. It's not possible to specify an -application-specific key.` - -func init() { - certCmd.Flags().BoolVarP(&overwriteCertificate, "force", "f", false, "Force overwrite of existing certificate and private key") - certCmd.MarkPersistentFlagRequired(applicationFlag) - deprecatedCertCmd.Flags().BoolVarP(&overwriteCertificate, "force", "f", false, "Force overwrite of existing certificate and private key") - deprecatedCertCmd.MarkPersistentFlagRequired(applicationFlag) -} - -func certExample() string { - return "$ vespa auth cert -a my-tenant.my-app.my-instance" -} - -var certCmd = &cobra.Command{ - Use: "cert", - Short: "Create a new private key and self-signed certificate for Vespa Cloud deployment", - Long: longDoc, - Example: certExample(), +application-specific key.`, + Example: `$ vespa auth cert -a my-tenant.my-app.my-instance +$ vespa auth cert -a my-tenant.my-app.my-instance path/to/application/package`, DisableAutoGenTag: true, SilenceUsage: true, Args: cobra.MaximumNArgs(1), @@ -63,9 +72,35 @@ var certCmd = &cobra.Command{ } var deprecatedCertCmd = &cobra.Command{ - Use: "cert", - Short: "Create a new private key and self-signed certificate for Vespa Cloud deployment", - Long: longDoc, + Use: "cert", + Short: "Create a new private key and self-signed certificate for Vespa Cloud deployment", + Long: `Create a new private key and self-signed certificate for Vespa Cloud deployment. + +The private key and certificate will be stored in the Vespa CLI home directory +(see 'vespa help config'). Other commands will then automatically load the +certificate as necessary. + +The certificate will be added to your application package specified as an +argument to this command (default '.'). + +It's possible to override the private key and certificate used through +environment variables. This can be useful in continuous integration systems. + +* VESPA_CLI_DATA_PLANE_CERT and VESPA_CLI_DATA_PLANE_KEY containing the + certificate and private key directly: + + export VESPA_CLI_DATA_PLANE_CERT="my cert" + export VESPA_CLI_DATA_PLANE_KEY="my private key" + +* VESPA_CLI_DATA_PLANE_CERT_FILE and VESPA_CLI_DATA_PLANE_KEY_FILE containing + paths to the certificate and private key: + + export VESPA_CLI_DATA_PLANE_CERT_FILE=/path/to/cert + export VESPA_CLI_DATA_PLANE_KEY_FILE=/path/to/key + +Note that when overriding key pair through environment variables, that key pair +will always be used for all applications. It's not possible to specify an +application-specific key.`, Example: "$ vespa cert -a my-tenant.my-app.my-instance", DisableAutoGenTag: true, SilenceUsage: true, @@ -75,14 +110,35 @@ var deprecatedCertCmd = &cobra.Command{ RunE: doCert, } +var certAddCmd = &cobra.Command{ + Use: "add", + Short: "Add certificate to application package", + Long: `Add an existing self-signed certificate for Vespa Cloud deployment to your application package. + +The certificate will be looked for in the Vespa CLI home directory (see 'vespa +help config') by default. + +The location of the application package can be specified as an argument to this +command (default '.').`, + Example: `$ vespa auth cert add -a my-tenant.my-app.my-instance +$ vespa auth cert add -a my-tenant.my-app.my-instance path/to/application/package`, + DisableAutoGenTag: true, + SilenceUsage: true, + Args: cobra.MinimumNArgs(1), + RunE: doCertAdd, +} + func doCert(_ *cobra.Command, args []string) error { app, err := getApplication() if err != nil { return err } - pkg, err := vespa.FindApplicationPackage(applicationSource(args), false) - if err != nil { - return err + var pkg vespa.ApplicationPackage + if !noApplicationPackage { + pkg, err = vespa.FindApplicationPackage(applicationSource(args), false) + if err != nil { + return err + } } cfg, err := LoadConfig() if err != nil { @@ -99,8 +155,10 @@ func doCert(_ *cobra.Command, args []string) error { if !overwriteCertificate { hint := "Use -f flag to force overwriting" - if pkg.HasCertificate() { - return errHint(fmt.Errorf("application package %s already contains a certificate", pkg.Path), hint) + if !noApplicationPackage { + if pkg.HasCertificate() { + return errHint(fmt.Errorf("application package %s already contains a certificate", pkg.Path), hint) + } } if util.PathExists(privateKeyFile) { return errHint(fmt.Errorf("private key %s already exists", color.Cyan(privateKeyFile)), hint) @@ -109,21 +167,26 @@ func doCert(_ *cobra.Command, args []string) error { return errHint(fmt.Errorf("certificate %s already exists", color.Cyan(certificateFile)), hint) } } - if pkg.IsZip() { - hint := "Try running 'mvn clean' before 'vespa auth cert', and then 'mvn package'" - return errHint(fmt.Errorf("cannot add certificate to compressed application package %s", pkg.Path), hint) + if !noApplicationPackage { + if pkg.IsZip() { + hint := "Try running 'mvn clean' before 'vespa auth cert', and then 'mvn package'" + return errHint(fmt.Errorf("cannot add certificate to compressed application package %s", pkg.Path), hint) + } } keyPair, err := vespa.CreateKeyPair() if err != nil { return err } - pkgCertificateFile := filepath.Join(pkg.Path, "security", "clients.pem") - if err := os.MkdirAll(filepath.Dir(pkgCertificateFile), 0755); err != nil { - return fmt.Errorf("could not create security directory: %w", err) - } - if err := keyPair.WriteCertificateFile(pkgCertificateFile, overwriteCertificate); err != nil { - return fmt.Errorf("could not write certificate to application package: %w", err) + var pkgCertificateFile string + if !noApplicationPackage { + pkgCertificateFile = filepath.Join(pkg.Path, "security", "clients.pem") + if err := os.MkdirAll(filepath.Dir(pkgCertificateFile), 0755); err != nil { + return fmt.Errorf("could not create security directory: %w", err) + } + if err := keyPair.WriteCertificateFile(pkgCertificateFile, overwriteCertificate); err != nil { + return fmt.Errorf("could not write certificate to application package: %w", err) + } } if err := keyPair.WriteCertificateFile(certificateFile, overwriteCertificate); err != nil { return fmt.Errorf("could not write certificate: %w", err) @@ -131,8 +194,64 @@ func doCert(_ *cobra.Command, args []string) error { if err := keyPair.WritePrivateKeyFile(privateKeyFile, overwriteCertificate); err != nil { return fmt.Errorf("could not write private key: %w", err) } - printSuccess("Certificate written to ", color.Cyan(pkgCertificateFile)) + if !noApplicationPackage { + printSuccess("Certificate written to ", color.Cyan(pkgCertificateFile)) + } printSuccess("Certificate written to ", color.Cyan(certificateFile)) printSuccess("Private key written to ", color.Cyan(privateKeyFile)) return nil } + +func doCertAdd(_ *cobra.Command, args []string) error { + app, err := getApplication() + if err != nil { + return err + } + pkg, err := vespa.FindApplicationPackage(applicationSource(args), false) + if err != nil { + return err + } + cfg, err := LoadConfig() + if err != nil { + return err + } + certificateFile, err := cfg.CertificatePath(app) + if err != nil { + return err + } + + if pkg.IsZip() { + hint := "Try running 'mvn clean' before 'vespa auth cert add', and then 'mvn package'" + return errHint(fmt.Errorf("unable to add certificate to compressed application package: %s", pkg.Path), hint) + } + + pkgCertificateFile := filepath.Join(pkg.Path, "security", "clients.pem") + if err := os.MkdirAll(filepath.Dir(pkgCertificateFile), 0755); err != nil { + return fmt.Errorf("could not create security directory: %w", err) + } + src, err := os.Open(certificateFile) + if errors.Is(err, os.ErrNotExist) { + return errHint(fmt.Errorf("there is not key pair generated for application '%s'", app), "Try running 'vespa auth cert' to generate it") + } else if err != nil { + return fmt.Errorf("could not open certificate file: %w", err) + } + defer src.Close() + flags := os.O_CREATE | os.O_RDWR + if overwriteCertificate { + flags |= os.O_TRUNC + } else { + flags |= os.O_EXCL + } + dst, err := os.OpenFile(pkgCertificateFile, flags, 0755) + if errors.Is(err, os.ErrExist) { + return errHint(fmt.Errorf("application package %s already contains a certificate", pkg.Path), "Use -f flag to force overwriting") + } else if err != nil { + return fmt.Errorf("could not open application certificate file for writing: %w", err) + } + if _, err := io.Copy(dst, src); err != nil { + return fmt.Errorf("could not copy certificate file to application: %w", err) + } + + printSuccess("Certificate written to ", color.Cyan(pkgCertificateFile)) + return nil +} diff --git a/client/go/cmd/cert_test.go b/client/go/cmd/cert_test.go index 4a115f54c65..51e99938d2b 100644 --- a/client/go/cmd/cert_test.go +++ b/client/go/cmd/cert_test.go @@ -9,6 +9,8 @@ import ( "path/filepath" "testing" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/assert" "github.com/vespa-engine/vespa/client/go/vespa" ) @@ -39,7 +41,7 @@ func testCert(t *testing.T, subcommand []string) { assert.Equal(t, fmt.Sprintf("Success: Certificate written to %s\nSuccess: Certificate written to %s\nSuccess: Private key written to %s\n", pkgCertificate, certificate, privateKey), out) args = append(subcommand, "-a", "t1.a1.i1", pkgDir) - _, outErr := execute(command{args: []string{"cert", "-a", "t1.a1.i1", pkgDir}, homeDir: homeDir}, t, nil) + _, outErr := execute(command{args: args, homeDir: homeDir}, t, nil) assert.Contains(t, outErr, fmt.Sprintf("Error: application package %s already contains a certificate", appDir)) } @@ -74,6 +76,44 @@ func testCertCompressedPackage(t *testing.T, subcommand []string) { assert.Contains(t, out, "Success: Private key written to") } +func TestCertAdd(t *testing.T) { + homeDir := filepath.Join(t.TempDir(), ".vespa") + execute(command{args: []string{"auth", "cert", "-N", "-a", "t1.a1.i1"}, homeDir: homeDir}, t, nil) + + pkgDir := mockApplicationPackage(t, false) + out, _ := execute(command{args: []string{"auth", "cert", "add", "-a", "t1.a1.i1", pkgDir}, homeDir: homeDir}, t, nil) + appDir := filepath.Join(pkgDir, "src", "main", "application") + pkgCertificate := filepath.Join(appDir, "security", "clients.pem") + assert.Equal(t, fmt.Sprintf("Success: Certificate written to %s\n", pkgCertificate), out) + + out, outErr := execute(command{args: []string{"auth", "cert", "add", "-a", "t1.a1.i1", pkgDir}, homeDir: homeDir}, t, nil) + assert.Equal(t, "", out) + assert.Contains(t, outErr, fmt.Sprintf("Error: application package %s already contains a certificate", appDir)) + out, _ = execute(command{args: []string{"auth", "cert", "add", "-f", "-a", "t1.a1.i1", pkgDir}, homeDir: homeDir}, t, nil) + assert.Equal(t, fmt.Sprintf("Success: Certificate written to %s\n", pkgCertificate), out) +} + +func TestCertNoAdd(t *testing.T) { + homeDir := filepath.Join(t.TempDir(), ".vespa") + out, _ := execute(command{args: []string{"auth", "cert", "-N", "-a", "t1.a1.i1"}, homeDir: homeDir}, t, nil) + + app, err := vespa.ApplicationFromString("t1.a1.i1") + assert.Nil(t, err) + + certificate := filepath.Join(homeDir, app.String(), "data-plane-public-cert.pem") + privateKey := filepath.Join(homeDir, app.String(), "data-plane-private-key.pem") + assert.Equal(t, fmt.Sprintf("Success: Certificate written to %s\nSuccess: Private key written to %s\n", certificate, privateKey), out) + + _, outErr := execute(command{args: []string{"auth", "cert", "-N", "-a", "t1.a1.i1"}, homeDir: homeDir}, t, nil) + assert.Contains(t, outErr, fmt.Sprintf("Error: private key %s already exists", privateKey)) + require.Nil(t, os.Remove(privateKey)) + _, outErr = execute(command{args: []string{"auth", "cert", "-N", "-a", "t1.a1.i1"}, homeDir: homeDir}, t, nil) + assert.Contains(t, outErr, fmt.Sprintf("Error: certificate %s already exists", certificate)) + + out, _ = execute(command{args: []string{"auth", "cert", "-N", "-f", "-a", "t1.a1.i1"}, homeDir: homeDir}, t, nil) + assert.Equal(t, fmt.Sprintf("Success: Certificate written to %s\nSuccess: Private key written to %s\n", certificate, privateKey), out) +} + func mockApplicationPackage(t *testing.T, java bool) string { dir := t.TempDir() appDir := filepath.Join(dir, "src", "main", "application") 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 a12b6a2ff69..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 } @@ -99,6 +95,8 @@ func execute(cmd command, t *testing.T, client *mockHttpClient) (string, string) queryCmd.Flags().VisitAll(resetFlag) documentCmd.Flags().VisitAll(resetFlag) logCmd.Flags().VisitAll(resetFlag) + certCmd.Flags().VisitAll(resetFlag) + certAddCmd.Flags().VisitAll(resetFlag) // Capture stdout and execute command var capturedOut bytes.Buffer @@ -120,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/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java index 13c5f978298..72e5a0dfc64 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java @@ -1190,7 +1190,7 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { request.getUri()).toString()); DeploymentStatus status = controller.jobController().deploymentStatus(application); - application.latestVersion().ifPresent(version -> toSlime(version, object.setObject("latestVersion"))); + application.latestVersion().ifPresent(version -> JobControllerApiHandlerHelper.toSlime(object.setObject("latestVersion"), version)); application.projectId().ifPresent(id -> object.setLong("projectId", id)); @@ -1314,7 +1314,6 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { request.getUri()).toString()); application.latestVersion().ifPresent(version -> { - sourceRevisionToSlime(version.source(), object.setObject("source")); version.sourceUrl().ifPresent(url -> object.setString("sourceUrl", url)); version.commit().ifPresent(commit -> object.setString("commit", commit)); }); @@ -1451,7 +1450,7 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { change.platform().ifPresent(version -> object.setString("version", version.toString())); change.application() .filter(version -> !version.isUnknown()) - .ifPresent(version -> toSlime(version, object.setObject("revision"))); + .ifPresent(version -> JobControllerApiHandlerHelper.toSlime(object.setObject("revision"), version)); } private void toSlime(Endpoint endpoint, Cursor object) { @@ -1503,7 +1502,6 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { .ifPresent(deploymentTimeToLive -> response.setLong("expiryTimeEpochMs", lastDeploymentStart.plus(deploymentTimeToLive).toEpochMilli())); application.projectId().ifPresent(i -> response.setString("screwdriverId", String.valueOf(i))); - sourceRevisionToSlime(deployment.applicationVersion().source(), response); var instance = application.instances().get(deploymentId.applicationId().instance()); if (instance != null) { @@ -1516,7 +1514,7 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { .map(type -> new JobId(instance.id(), type)) .map(status.jobSteps()::get) .ifPresent(stepStatus -> { - JobControllerApiHandlerHelper.applicationVersionToSlime( + JobControllerApiHandlerHelper.toSlime( response.setObject("applicationVersion"), deployment.applicationVersion()); if (!status.jobsToRun().containsKey(stepStatus.job().get())) response.setString("status", "complete"); @@ -1564,23 +1562,6 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { .stream().findFirst().orElse(deployment.at()); } - private void toSlime(ApplicationVersion applicationVersion, Cursor object) { - if ( ! applicationVersion.isUnknown()) { - object.setLong("buildNumber", applicationVersion.buildNumber().getAsLong()); - object.setString("hash", applicationVersion.id()); - sourceRevisionToSlime(applicationVersion.source(), object.setObject("source")); - applicationVersion.sourceUrl().ifPresent(url -> object.setString("sourceUrl", url)); - applicationVersion.commit().ifPresent(commit -> object.setString("commit", commit)); - } - } - - private void sourceRevisionToSlime(Optional<SourceRevision> revision, Cursor object) { - if (revision.isEmpty()) return; - object.setString("gitRepository", revision.get().repository()); - object.setString("gitBranch", revision.get().branch()); - object.setString("gitCommit", revision.get().commit()); - } - private void toSlime(RotationState state, Cursor object) { Cursor bcpStatus = object.setObject("bcpStatus"); bcpStatus.setString("rotationStatus", rotationStateString(state)); @@ -2519,7 +2500,7 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { object.setLong("id", run.id().number()); object.setString("version", run.versions().targetPlatform().toFullString()); if ( ! run.versions().targetApplication().isUnknown()) - toSlime(run.versions().targetApplication(), object.setObject("revision")); + JobControllerApiHandlerHelper.toSlime(object.setObject("revision"), run.versions().targetApplication()); object.setString("reason", "unknown reason"); object.setLong("at", run.end().orElse(run.start()).toEpochMilli()); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java index 659cfc6e39d..648d03a54d7 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java @@ -102,22 +102,6 @@ class JobControllerApiHandlerHelper { return new SlimeJsonResponse(slime); } - static void applicationVersionToSlime(Cursor versionObject, ApplicationVersion version) { - versionObject.setString("hash", version.id()); - if (version.isUnknown()) - return; - - versionObject.setLong("build", version.buildNumber().getAsLong()); - Cursor sourceObject = versionObject.setObject("source"); - version.source().ifPresent(source -> { - sourceObject.setString("gitRepository", source.repository()); - sourceObject.setString("gitBranch", source.branch()); - sourceObject.setString("gitCommit", source.commit()); - }); - version.sourceUrl().ifPresent(url -> versionObject.setString("sourceUrl", url)); - version.commit().ifPresent(commit -> versionObject.setString("commit", commit)); - } - /** * @return Response with logs from a single run */ @@ -380,12 +364,12 @@ class JobControllerApiHandlerHelper { } Cursor buildsArray = responseObject.setArray("builds"); - application.versions().stream().sorted(reverseOrder()).forEach(version -> applicationVersionToSlime(buildsArray.addObject(), version)); + application.versions().stream().sorted(reverseOrder()).forEach(version -> toSlime(buildsArray.addObject(), version)); return new SlimeJsonResponse(slime); } - private static void toSlime(Cursor versionObject, ApplicationVersion version) { + static void toSlime(Cursor versionObject, ApplicationVersion version) { version.buildNumber().ifPresent(id -> versionObject.setLong("build", id)); version.compileVersion().ifPresent(platform -> versionObject.setString("compileVersion", platform.toFullString())); version.sourceUrl().ifPresent(url -> versionObject.setString("sourceUrl", url)); 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/persistence/testdata/complete-application.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/complete-application.json index 19df8059cf8..29f748d5408 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/complete-application.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/testdata/complete-application.json @@ -6,12 +6,7 @@ "validationOverrides": "<validation-overrides>\n <allow until=\"2016-04-28\" comment=\"Renaming content cluster\">content-cluster-removal</allow>\n <allow until=\"2016-08-22\" comment=\"Migrating us-east-3 to C-2E\">cluster-size-reduction</allow>\n <allow until=\"2017-06-30\" comment=\"Test Vespa upgrade tests\">force-automatic-tenant-upgrade-test</allow>\n</validation-overrides>\n", "projectId": 102889, "deployingField": { - "buildNumber": 42, - "sourceRevision": { - "repositoryField": "git@git.host:user/repo.git", - "branchField": "origin/master", - "commitField": "234f3e4e77049d0b9538c9e1b356d17eb1dedb6a" - } + "build": 42 }, "outstandingChangeField": false, "queryQuality": 100, diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application2-with-patches.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application2-with-patches.json index df3f9699677..c8f5b7bf50a 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application2-with-patches.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application2-with-patches.json @@ -3,13 +3,8 @@ "application": "application2", "deployments": "http://localhost:8080/application/v4/tenant/tenant2/application/application2/job/", "latestVersion": { - "buildNumber": 1, - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, + "build": 1, + "compileVersion": "6.1.0", "sourceUrl": "repository1/tree/commit1", "commit": "commit1" }, @@ -24,13 +19,8 @@ "instance": "instance1", "deploying": { "revision": { - "buildNumber": 1, - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, + "build": 1, + "compileVersion": "6.1.0", "sourceUrl": "repository1/tree/commit1", "commit": "commit1" } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application2.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application2.json index 9ef46247629..7aae1815dac 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application2.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application2.json @@ -3,13 +3,8 @@ "application": "application2", "deployments": "http://localhost:8080/application/v4/tenant/tenant2/application/application2/job/", "latestVersion": { - "buildNumber": 1, - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, + "build": 1, + "compileVersion": "6.1.0", "sourceUrl": "repository1/tree/commit1", "commit": "commit1" }, @@ -23,13 +18,8 @@ "instance": "instance1", "deploying": { "revision": { - "buildNumber": 1, - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, + "build": 1, + "compileVersion": "6.1.0", "sourceUrl": "repository1/tree/commit1", "commit": "commit1" } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-cloud.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-cloud.json index fd4093ca332..74f41524d3e 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-cloud.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-cloud.json @@ -21,17 +21,9 @@ "revision": "1.0.1-commit1", "deployTimeEpochMs": "(ignore)", "screwdriverId": "1000", - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1", "applicationVersion": { - "hash": "1.0.1-commit1", "build": 1, - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, + "compileVersion": "6.1.0", "sourceUrl": "repository1/tree/commit1", "commit": "commit1" }, 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 55b3a8b388f..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": [ @@ -1232,35 +1218,20 @@ ], "builds": [ { - "hash": "1.0.3-commit1", "build": 3, - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, + "compileVersion": "6.1.0", "sourceUrl": "repository1/tree/commit1", "commit": "commit1" }, { - "hash": "1.0.2-commit1", "build": 2, - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, + "compileVersion": "6.1.0", "sourceUrl": "repository1/tree/commit1", "commit": "commit1" }, { - "hash": "1.0.1-commit1", "build": 1, - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, + "compileVersion": "6.1.0", "sourceUrl": "repository1/tree/commit1", "commit": "commit1" } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview.json index da1ccbd56e9..e896a2feba1 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview.json @@ -678,46 +678,26 @@ ], "builds": [ { - "hash": "1.0.4-commit1", "build": 4, - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, + "compileVersion": "6.1.0", "sourceUrl": "repository1/tree/commit1", "commit": "commit1" }, { - "hash": "1.0.3-commit1", "build": 3, - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, + "compileVersion": "6.1.0", "sourceUrl": "repository1/tree/commit1", "commit": "commit1" }, { - "hash": "1.0.2-commit1", "build": 2, - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, + "compileVersion": "6.1.0", "sourceUrl": "repository1/tree/commit1", "commit": "commit1" }, { - "hash": "1.0.1-commit1", "build": 1, - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, + "compileVersion": "6.1.0", "sourceUrl": "repository1/tree/commit1", "commit": "commit1" } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-with-routing-policy.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-with-routing-policy.json index 4457bede34e..3f70ae1e303 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-with-routing-policy.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-with-routing-policy.json @@ -21,17 +21,9 @@ "revision": "1.0.1-commit1", "deployTimeEpochMs": "(ignore)", "screwdriverId": "1000", - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1", "applicationVersion": { - "hash": "1.0.1-commit1", "build": 1, - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, + "compileVersion": "6.1.0", "sourceUrl": "repository1/tree/commit1", "commit": "commit1" }, diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-without-shared-endpoints.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-without-shared-endpoints.json index 39b8c779184..860fe541682 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-without-shared-endpoints.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-without-shared-endpoints.json @@ -21,17 +21,9 @@ "revision": "1.0.1-commit1", "deployTimeEpochMs": "(ignore)", "screwdriverId": "1000", - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1", "applicationVersion": { - "hash": "1.0.1-commit1", "build": 1, - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, + "compileVersion": "6.1.0", "sourceUrl": "repository1/tree/commit1", "commit": "commit1" }, diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment.json index 621617f1b1c..315b1af25c7 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment.json @@ -37,9 +37,6 @@ "revision": "(ignore)", "deployTimeEpochMs": "(ignore)", "screwdriverId": "123", - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1", "endpointStatus": [ { "endpointId": "default", @@ -50,13 +47,8 @@ } ], "applicationVersion": { - "hash": "1.0.1-commit1", "build": 1, - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, + "compileVersion": "6.1.0", "sourceUrl": "repository1/tree/commit1", "commit": "commit1" }, diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance-with-routing-policy.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance-with-routing-policy.json index e3f70e84f43..afac12a191b 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance-with-routing-policy.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance-with-routing-policy.json @@ -3,11 +3,6 @@ "application": "application1", "instance": "instance1", "deployments": "http://localhost:8080/application/v4/tenant/tenant1/application/application1/instance/instance1/job/", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, "sourceUrl": "repository1/tree/commit1", "commit": "commit1", "projectId": 1000, diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance.json index bcbdf448ad5..b98de97856d 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance.json @@ -3,23 +3,13 @@ "application": "application1", "instance": "instance1", "deployments": "http://localhost:8080/application/v4/tenant/tenant1/application/application1/instance/instance1/job/", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, "sourceUrl": "repository1/tree/commit1", "commit": "commit1", "projectId": 123, "deploying": { "revision": { - "buildNumber": 1, - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, + "build": 1, + "compileVersion": "6.1.0", "sourceUrl": "repository1/tree/commit1", "commit": "commit1" } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance1-recursive.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance1-recursive.json index 0c4f046f45c..a697c667ab0 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance1-recursive.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/instance1-recursive.json @@ -3,23 +3,13 @@ "application": "application1", "instance": "instance1", "deployments": "http://localhost:8080/application/v4/tenant/tenant1/application/application1/instance/instance1/job/", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, "sourceUrl": "repository1/tree/commit1", "commit": "commit1", "projectId": 123, "deploying": { "revision": { - "buildNumber": 1, - "hash": "1.0.1-commit1", - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, + "build": 1, + "compileVersion": "6.1.0", "sourceUrl": "repository1/tree/commit1", "commit": "commit1" } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/prod-us-central-1.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/prod-us-central-1.json index d9ec8e4dfef..2daa7f54cf6 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/prod-us-central-1.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/prod-us-central-1.json @@ -40,9 +40,6 @@ "revision": "(ignore)", "deployTimeEpochMs": "(ignore)", "screwdriverId": "123", - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1", "endpointStatus": [ { "endpointId": "default", @@ -53,13 +50,8 @@ } ], "applicationVersion": { - "hash": "1.0.1-commit1", "build": 1, - "source": { - "gitRepository": "repository1", - "gitBranch": "master", - "gitCommit": "commit1" - }, + "compileVersion": "6.1.0", "sourceUrl": "repository1/tree/commit1", "commit": "commit1" }, 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/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); |