From c0716ebab991329ce44bc1624224428db01de5d3 Mon Sep 17 00:00:00 2001
From: olaughter <fred@climatepolicyradar.org>
Date: Fri, 19 Jul 2024 08:26:53 +0100
Subject: [PATCH 1/4] Add method to Services to parse cert paths
---
client/go/internal/vespa/xml/config.go | 31 ++++++++++++--
client/go/internal/vespa/xml/config_test.go | 45 +++++++++++++++++++++
2 files changed, 73 insertions(+), 3 deletions(-)
diff --git a/client/go/internal/vespa/xml/config.go b/client/go/internal/vespa/xml/config.go
index d1c16b016523..47e102867cf7 100644
--- a/client/go/internal/vespa/xml/config.go
+++ b/client/go/internal/vespa/xml/config.go
@@ -75,9 +75,10 @@ type Services struct {
}
type Container struct {
- Root xml.Name `xml:"container"`
- ID string `xml:"id,attr"`
- Nodes Nodes `xml:"nodes"`
+ Root xml.Name `xml:"container"`
+ ID string `xml:"id,attr"`
+ Nodes Nodes `xml:"nodes"`
+ Clients Clients `xml:"clients"`
}
type Content struct {
@@ -85,6 +86,19 @@ type Content struct {
Nodes Nodes `xml:"nodes"`
}
+type Clients struct {
+ Client []Client `xml:"client"`
+}
+
+type Client struct {
+ Id string `xml:"id,attr"`
+ Certificate Certificate `xml:"certificate"`
+}
+
+type Certificate struct {
+ File string `xml:"file,attr"`
+}
+
type Nodes struct {
Count string `xml:"count,attr"`
Resources *Resources `xml:"resources,omitempty"`
@@ -98,6 +112,17 @@ type Resources struct {
func (s Services) String() string { return s.rawXML.String() }
+// Reads file paths from services.xml
+func (s Services) CertPaths() []string {
+ var certificates []string
+ for _, container := range s.Container {
+ for _, client := range container.Clients.Client {
+ certificates = append(certificates, client.Certificate.File)
+ }
+ }
+ return certificates
+}
+
// Replace replaces any elements of name found under parentName with data.
func (s *Services) Replace(parentName, name string, data interface{}) error {
rewritten, err := Replace(&s.rawXML, parentName, name, data)
diff --git a/client/go/internal/vespa/xml/config_test.go b/client/go/internal/vespa/xml/config_test.go
index e17a608b5545..f4a9064f26a3 100644
--- a/client/go/internal/vespa/xml/config_test.go
+++ b/client/go/internal/vespa/xml/config_test.go
@@ -243,6 +243,51 @@ func TestReadServicesNoResources(t *testing.T) {
}
}
+func TestReadClientPaths(t *testing.T) {
+ s := `
+<services xmlns:deploy="vespa" xmlns:preprocess="properties">
+ <container id="qrs">
+ <clients>
+ <client id="test" permissions="read">
+ <certificate file="security/test.pem"/>
+ </client>
+ <client id="other" permissions="read">
+ <certificate file="security/other.pem"/>
+ </client>
+ </clients>
+ </container>
+</services>
+`
+ services, err := ReadServices(strings.NewReader(s))
+ if err != nil {
+ t.Fatal(err)
+ }
+ certPaths := services.CertPaths()
+ if got := certPaths[0]; got != "security/test.pem" {
+ t.Errorf("got %+v, want security/test.pem", got)
+ }
+ if got := certPaths[1]; got != "security/other.pem" {
+ t.Errorf("got %+v, want security/other.pem", got)
+ }
+}
+
+func TestReadNoClientPaths(t *testing.T) {
+ s := `
+<services xmlns:deploy="vespa" xmlns:preprocess="properties">
+ <container id="qrs">
+ </container>
+</services>
+`
+ services, err := ReadServices(strings.NewReader(s))
+ if err != nil {
+ t.Fatal(err)
+ }
+ certPaths := services.CertPaths()
+ if got := certPaths; got != nil {
+ t.Errorf("got %+v, want nil", got)
+ }
+}
+
func TestParseResources(t *testing.T) {
assertResources(t, "foo", Resources{}, true)
assertResources(t, "vcpu=2,memory=4Gb", Resources{}, true)
From 3d936eb9c7307089c2b213df3a4199516224d1fe Mon Sep 17 00:00:00 2001
From: olaughter <fred@climatepolicyradar.org>
Date: Fri, 19 Jul 2024 08:32:10 +0100
Subject: [PATCH 2/4] Extend cert check to include services.xml clients
---
client/go/internal/vespa/application.go | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/client/go/internal/vespa/application.go b/client/go/internal/vespa/application.go
index d499006c9820..973505218316 100644
--- a/client/go/internal/vespa/application.go
+++ b/client/go/internal/vespa/application.go
@@ -20,7 +20,22 @@ type ApplicationPackage struct {
TestPath string
}
-func (ap *ApplicationPackage) HasCertificate() bool { return ap.hasFile("security", "clients.pem") }
+func (ap *ApplicationPackage) HasCertificate(certPaths []string) bool {
+ if len(certPaths) == 0 {
+ if ap.hasFile("security", "clients.pem") {
+ return true
+ } else {
+ return false
+ }
+ } else {
+ for _, certPath := range certPaths {
+ if ap.hasFile(certPath) {
+ return true
+ }
+ }
+ return false
+ }
+}
func (ap *ApplicationPackage) HasMatchingCertificate(certificatePEM []byte) (bool, error) {
clientsPEM, err := os.ReadFile(filepath.Join(ap.Path, "security", "clients.pem"))
From 8df46e969cd14b3dd62180cf2a5f0ced1ac4291c Mon Sep 17 00:00:00 2001
From: olaughter <fred@climatepolicyradar.org>
Date: Fri, 19 Jul 2024 11:40:40 +0100
Subject: [PATCH 3/4] Add services xml argument
The `HasCertificate` check now requires the parsed services.xml object.
---
client/go/internal/cli/cmd/cert.go | 9 +++++++--
client/go/internal/cli/cmd/deploy.go | 9 ++++++++-
client/go/internal/cli/cmd/prod.go | 7 ++++++-
client/go/internal/cli/cmd/prod_test.go | 10 ++++++++++
client/go/internal/vespa/deploy.go | 12 ++++++------
client/go/internal/vespa/deploy_test.go | 10 +++++-----
6 files changed, 42 insertions(+), 15 deletions(-)
diff --git a/client/go/internal/cli/cmd/cert.go b/client/go/internal/cli/cmd/cert.go
index 9e0b8f6805cc..dcf0ad6a7f5f 100644
--- a/client/go/internal/cli/cmd/cert.go
+++ b/client/go/internal/cli/cmd/cert.go
@@ -157,7 +157,8 @@ func doCertAdd(cli *CLI, overwriteCertificate bool, args []string) error {
if err != nil {
return err
}
- if pkg.HasCertificate() && !overwriteCertificate {
+ defaultCertPath := []string{"security/clients.pem"}
+ if pkg.HasCertificate(defaultCertPath) && !overwriteCertificate {
return errHint(fmt.Errorf("application package '%s' already contains a certificate", pkg.Path), "Use -f flag to force overwriting")
}
return requireCertificate(true, false, cli, target, pkg)
@@ -182,7 +183,11 @@ func requireCertificate(force, ignoreZip bool, cli *CLI, target vespa.Target, pk
if force {
return copyCertificate(tlsOptions, cli, pkg)
}
- if pkg.HasCertificate() {
+ servicesXML, err := readServicesXML(pkg)
+ if err != nil {
+ return fmt.Errorf("Error reading services.xml: %w", err)
+ }
+ if pkg.HasCertificate(servicesXML.CertPaths()) {
if cli.isCI() {
return nil // A matching certificate is not required in CI environments
}
diff --git a/client/go/internal/cli/cmd/deploy.go b/client/go/internal/cli/cmd/deploy.go
index 19c48f942a66..dc6a511e3370 100644
--- a/client/go/internal/cli/cmd/deploy.go
+++ b/client/go/internal/cli/cmd/deploy.go
@@ -81,8 +81,15 @@ $ vespa deploy -t cloud -z perf.aws-us-east-1c`,
return err
}
var result vespa.PrepareResult
+ var certPaths []string
+ servicesXML, err := readServicesXML(pkg)
+ if err != nil {
+ certPaths = []string{}
+ } else {
+ certPaths = servicesXML.CertPaths()
+ }
err = cli.spinner(cli.Stderr, "Uploading application package...", func() error {
- result, err = vespa.Deploy(opts)
+ result, err = vespa.Deploy(opts, certPaths)
return err
})
if err != nil {
diff --git a/client/go/internal/cli/cmd/prod.go b/client/go/internal/cli/cmd/prod.go
index 139e4690ed2e..b4476ba3b295 100644
--- a/client/go/internal/cli/cmd/prod.go
+++ b/client/go/internal/cli/cmd/prod.go
@@ -165,7 +165,12 @@ $ vespa prod deploy`,
AuthorEmail: options.authorEmail,
SourceURL: options.sourceURL,
}
- build, err := vespa.Submit(deployment, submission)
+
+ servicesXML, err := readServicesXML(pkg)
+ if err != nil {
+ return fmt.Errorf("Error reading services.xml: %w", err)
+ }
+ build, err := vespa.Submit(deployment, submission, servicesXML.CertPaths())
if err != nil {
return fmt.Errorf("could not deploy application: %w", err)
} else {
diff --git a/client/go/internal/cli/cmd/prod_test.go b/client/go/internal/cli/cmd/prod_test.go
index 8ea20b3bbe50..c1d73c7c13e0 100644
--- a/client/go/internal/cli/cmd/prod_test.go
+++ b/client/go/internal/cli/cmd/prod_test.go
@@ -61,6 +61,11 @@ func TestProdInit(t *testing.T) {
containerFragment := `<container id="qrs" version="1.0">
<document-api></document-api>
<search></search>
+ <clients>
+ <client id="test" permissions="read">
+ <certificate file="security/clients.pem"></certificate>
+ </client>
+ </clients>
<nodes count="4"></nodes>
</container>`
assert.Contains(t, servicesXML, containerFragment)
@@ -111,6 +116,11 @@ func createApplication(t *testing.T, pkgDir string, java bool, skipTests bool) {
<container id="qrs" version="1.0">
<document-api/>
<search/>
+ <clients>
+ <client id="test" permissions="read">
+ <certificate file="security/clients.pem"/>
+ </client>
+ </clients>
<nodes count="2">
<resources vcpu="4" memory="8Gb" disk="100Gb"/>
</nodes>
diff --git a/client/go/internal/vespa/deploy.go b/client/go/internal/vespa/deploy.go
index 2c96b8b09352..84e68f24c19a 100644
--- a/client/go/internal/vespa/deploy.go
+++ b/client/go/internal/vespa/deploy.go
@@ -334,13 +334,13 @@ func Deactivate(deployment DeploymentOptions) error {
}
// Deploy deploys an application.
-func Deploy(deployment DeploymentOptions) (PrepareResult, error) {
+func Deploy(deployment DeploymentOptions, certPaths []string) (PrepareResult, error) {
var (
u *url.URL
err error
)
if deployment.Target.IsCloud() {
- if err := checkDeploymentOpts(deployment); err != nil {
+ if err := checkDeploymentOpts(deployment, certPaths); err != nil {
return PrepareResult{}, err
}
if deployment.Target.Deployment().Zone.Environment == "" || deployment.Target.Deployment().Zone.Region == "" {
@@ -373,11 +373,11 @@ func copyToPart(dst *multipart.Writer, src io.Reader, fieldname, filename string
return nil
}
-func Submit(opts DeploymentOptions, submission Submission) (int64, error) {
+func Submit(opts DeploymentOptions, submission Submission, certPaths []string) (int64, error) {
if !opts.Target.IsCloud() {
return 0, fmt.Errorf("%s: deploy is unsupported by %s target", opts, opts.Target.Type())
}
- if err := checkDeploymentOpts(opts); err != nil {
+ if err := checkDeploymentOpts(opts, certPaths); err != nil {
return 0, err
}
submitURL := opts.Target.Deployment().System.SubmitURL(opts.Target.Deployment())
@@ -449,8 +449,8 @@ func deployServiceDo(request *http.Request, timeout time.Duration, opts Deployme
return s.Do(request, timeout)
}
-func checkDeploymentOpts(opts DeploymentOptions) error {
- if opts.Target.Type() == TargetCloud && !opts.ApplicationPackage.HasCertificate() {
+func checkDeploymentOpts(opts DeploymentOptions, certPaths []string) error {
+ if opts.Target.Type() == TargetCloud && !opts.ApplicationPackage.HasCertificate(certPaths) {
return fmt.Errorf("%s: missing certificate in package", opts)
}
if !opts.Target.IsCloud() && !opts.Version.IsZero() {
diff --git a/client/go/internal/vespa/deploy_test.go b/client/go/internal/vespa/deploy_test.go
index 4c2fb9122246..3a5c14fbfa0e 100644
--- a/client/go/internal/vespa/deploy_test.go
+++ b/client/go/internal/vespa/deploy_test.go
@@ -27,7 +27,7 @@ func TestDeploy(t *testing.T) {
Target: target,
ApplicationPackage: ApplicationPackage{Path: appDir},
}
- _, err := Deploy(opts)
+ _, err := Deploy(opts, []string{})
assert.Nil(t, err)
assert.Equal(t, 1, len(httpClient.Requests))
req := httpClient.LastRequest
@@ -49,7 +49,7 @@ func TestDeployCloud(t *testing.T) {
Target: target,
ApplicationPackage: ApplicationPackage{Path: appDir},
}
- _, err := Deploy(opts)
+ _, err := Deploy(opts, []string{})
require.Nil(t, err)
assert.Equal(t, 1, len(httpClient.Requests))
req := httpClient.LastRequest
@@ -62,7 +62,7 @@ func TestDeployCloud(t *testing.T) {
assert.False(t, hasDeployOptions)
opts.Version = version.MustParse("1.2.3")
- _, err = Deploy(opts)
+ _, err = Deploy(opts, []string{})
require.Nil(t, err)
req = httpClient.LastRequest
values = parseMultiPart(t, req)
@@ -83,7 +83,7 @@ func TestSubmit(t *testing.T) {
ApplicationPackage: ApplicationPackage{Path: appDir},
}
httpClient.NextResponseString(200, `{"build": 42}`)
- build, err := Submit(opts, Submission{})
+ build, err := Submit(opts, Submission{}, []string{})
require.Nil(t, err)
require.Equal(t, int64(42), build)
require.Nil(t, httpClient.LastRequest.ParseMultipartForm(1<<20))
@@ -102,7 +102,7 @@ func TestSubmit(t *testing.T) {
Description: "broken garbage",
AuthorEmail: "foo@example.com",
SourceURL: "https://github.com/foo/repo",
- })
+ }, []string{})
require.Nil(t, err)
require.Equal(t, int64(43), build)
require.Nil(t, httpClient.LastRequest.ParseMultipartForm(1<<20))
From 5b7dbbd62275186b69ee70faabe0edf6ffdf3358 Mon Sep 17 00:00:00 2001
From: olaughter <fred@climatepolicyradar.org>
Date: Fri, 19 Jul 2024 11:44:57 +0100
Subject: [PATCH 4/4] ignore vs code launch config
---
.gitignore | 1 +
1 file changed, 1 insertion(+)
diff --git a/.gitignore b/.gitignore
index 1de1c8e7b418..e315bbe907c4 100644
--- a/.gitignore
+++ b/.gitignore
@@ -55,3 +55,4 @@ install_manifest.txt
*.cbp
!/.copr/Makefile
!/.buildkite/Makefile
+.vscode/launch.json