-
Notifications
You must be signed in to change notification settings - Fork 700
feat: automatically adds acces to the vault when deploying dev #36116
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
3f14cff
9635f6f
5fe685a
5787b81
1719a30
f655b02
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,158 @@ | ||||||||||||||||||||||||||||||||||||||||||
| // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. | ||||||||||||||||||||||||||||||||||||||||||
| package vespa | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| import ( | ||||||||||||||||||||||||||||||||||||||||||
| "bytes" | ||||||||||||||||||||||||||||||||||||||||||
| "encoding/json" | ||||||||||||||||||||||||||||||||||||||||||
| "fmt" | ||||||||||||||||||||||||||||||||||||||||||
| "io" | ||||||||||||||||||||||||||||||||||||||||||
| "net/http" | ||||||||||||||||||||||||||||||||||||||||||
| "time" | ||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| const SECRET_STORE_DEV_ALIAS = "SANDBOX" | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| type vaultAccessRule struct { | ||||||||||||||||||||||||||||||||||||||||||
| Application string `json:"application"` | ||||||||||||||||||||||||||||||||||||||||||
| Contexts []string `json:"contexts"` | ||||||||||||||||||||||||||||||||||||||||||
| ID int `json:"id"` | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| type vaultResponse struct { | ||||||||||||||||||||||||||||||||||||||||||
| Rules []vaultAccessRule `json:"rules"` | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| func (t *cloudTarget) vaultAccessURL(tenant, vaultName string) string { | ||||||||||||||||||||||||||||||||||||||||||
| return fmt.Sprintf("%s/tenant-secret/v1/tenant/%s/vault/%s", t.apiOptions.System.URL, tenant, vaultName) | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| func (t *cloudTarget) csrfToken() (string, error) { | ||||||||||||||||||||||||||||||||||||||||||
| req, err := http.NewRequest("GET", fmt.Sprintf("%s/csrf/v1", t.apiOptions.System.URL), nil) | ||||||||||||||||||||||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||||||||||||||||||||||
| return "", err | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| deployService, err := t.DeployService() | ||||||||||||||||||||||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||||||||||||||||||||||
| return "", err | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| resp, err := deployService.Do(req, 10*time.Second) | ||||||||||||||||||||||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||||||||||||||||||||||
| return "", err | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| defer resp.Body.Close() | ||||||||||||||||||||||||||||||||||||||||||
| var result struct { | ||||||||||||||||||||||||||||||||||||||||||
| Token string `json:"token"` | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { | ||||||||||||||||||||||||||||||||||||||||||
| return "", err | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| return result.Token, nil | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+30
to
+54
|
||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| func (t *cloudTarget) ensureVaultAccessRule(vaultName string) error { | ||||||||||||||||||||||||||||||||||||||||||
| deployment := t.deploymentOptions.Deployment | ||||||||||||||||||||||||||||||||||||||||||
| tenant := deployment.Application.Tenant | ||||||||||||||||||||||||||||||||||||||||||
| appID := deployment.Application.Application // just the application name; tenant is already in the URL path | ||||||||||||||||||||||||||||||||||||||||||
| vaultURL := t.vaultAccessURL(tenant, vaultName) | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| // GET existing rules | ||||||||||||||||||||||||||||||||||||||||||
| req, err := http.NewRequest("GET", vaultURL, nil) | ||||||||||||||||||||||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||||||||||||||||||||||
| return err | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| deployService, err := t.DeployService() | ||||||||||||||||||||||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||||||||||||||||||||||
| return err | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| resp, err := deployService.Do(req, 10*time.Second) | ||||||||||||||||||||||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||||||||||||||||||||||
| return fmt.Errorf("could not get vault access rules for %q: %w", vaultName, err) | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| defer resp.Body.Close() | ||||||||||||||||||||||||||||||||||||||||||
| getRawBody, _ := io.ReadAll(resp.Body) | ||||||||||||||||||||||||||||||||||||||||||
| var vaultResp vaultResponse | ||||||||||||||||||||||||||||||||||||||||||
| if err := json.Unmarshal(getRawBody, &vaultResp); err != nil { | ||||||||||||||||||||||||||||||||||||||||||
| return fmt.Errorf("could not parse vault access rules for %q: %w", vaultName, err) | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+72
to
+85
|
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| // Check if access rule already exists for this application with the dev alias | ||||||||||||||||||||||||||||||||||||||||||
| for _, rule := range vaultResp.Rules { | ||||||||||||||||||||||||||||||||||||||||||
| if rule.Application == appID { | ||||||||||||||||||||||||||||||||||||||||||
| for _, ctx := range rule.Contexts { | ||||||||||||||||||||||||||||||||||||||||||
| if ctx == SECRET_STORE_DEV_ALIAS { | ||||||||||||||||||||||||||||||||||||||||||
| return nil | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| // Build new rule with no context restriction (grants access to all environments) | ||||||||||||||||||||||||||||||||||||||||||
| newRule := vaultAccessRule{ | ||||||||||||||||||||||||||||||||||||||||||
| Application: appID, | ||||||||||||||||||||||||||||||||||||||||||
| Contexts: []string{SECRET_STORE_DEV_ALIAS}, | ||||||||||||||||||||||||||||||||||||||||||
| ID: len(vaultResp.Rules), | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
| updatedRules := vaultResponse{Rules: append(vaultResp.Rules, newRule)} | ||||||||||||||||||||||||||||||||||||||||||
| body, err := json.Marshal(updatedRules) | ||||||||||||||||||||||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||||||||||||||||||||||
| return err | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| csrfToken, _ := t.csrfToken() | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| // PUT updated rules | ||||||||||||||||||||||||||||||||||||||||||
| putReq, err := http.NewRequest("PUT", vaultURL, bytes.NewReader(body)) | ||||||||||||||||||||||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||||||||||||||||||||||
| return err | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| putReq.Header.Set("Content-Type", "application/json") | ||||||||||||||||||||||||||||||||||||||||||
| if csrfToken != "" { | ||||||||||||||||||||||||||||||||||||||||||
| putReq.Header.Set("vespa-csrf-token", csrfToken) | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
| deployService2, err := t.DeployService() | ||||||||||||||||||||||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||||||||||||||||||||||
| return err | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| putResp, err := deployService2.Do(putReq, 10*time.Second) | ||||||||||||||||||||||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||||||||||||||||||||||
| return fmt.Errorf("could not set vault access rule for %q: %w", vaultName, err) | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| defer putResp.Body.Close() | ||||||||||||||||||||||||||||||||||||||||||
| putRawBody, _ := io.ReadAll(putResp.Body) | ||||||||||||||||||||||||||||||||||||||||||
| var putVaultResp vaultResponse | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
| var putVaultResp vaultResponse |
Outdated
Copilot
AI
Mar 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PUT response status code is not checked before attempting to read/unmarshal the body. If the API returns a non-2xx status, this may turn into a JSON parsing error and hide the actual server error. Check putResp.StatusCode and return a clear error (include body) when it’s not successful.
Outdated
Copilot
AI
Mar 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After the PUT, the confirmation check iterates over vaultResp.Rules (the pre-PUT response) instead of the PUT response. If the rule wasn’t present before, this will always fall through and return an error even when the PUT succeeded. Iterate over putVaultResp.Rules (or re-GET) when verifying the rule was applied.
Copilot
AI
Mar 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EnsureVaultAccessForDev is documented/named as applying to dev deployments, but it currently runs for any cloud deployment (including prod) as long as vaultNames is non-empty. Add a guard on target.Deployment().Zone.Environment (or equivalent) so this only mutates SANDBOX/dev access when deploying to dev.
| // Errors are non-fatal warnings for the caller. | |
| func EnsureVaultAccessForDev(target Target, vaultNames []string) error { | |
| ct, ok := target.(*cloudTarget) | |
| if !ok || len(vaultNames) == 0 { | |
| return nil | |
| // Returns nil for non-cloud targets, non-dev environments, or when no vaults are referenced. | |
| // Errors are non-fatal warnings for the caller. | |
| func EnsureVaultAccessForDev(target Target, vaultNames []string) error { | |
| if len(vaultNames) == 0 { | |
| return nil | |
| } | |
| deployment := target.Deployment() | |
| if deployment == nil || deployment.Zone.Environment != "dev" { | |
| // Only modify SANDBOX/dev access rules when deploying to dev. | |
| return nil | |
| } | |
| ct, ok := target.(*cloudTarget) | |
| if !ok { |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,85 @@ | ||
| // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. | ||
| package vespa | ||
|
|
||
| import ( | ||
| "encoding/json" | ||
| "io" | ||
| "net/http" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
| "github.com/vespa-engine/vespa/client/go/internal/mock" | ||
| ) | ||
|
|
||
| func TestEnsureVaultAccessForDevNonCloud(t *testing.T) { | ||
| // Non-cloud targets are a no-op | ||
| client := &mock.HTTPClient{} | ||
| lt := LocalTarget(client, TLSOptions{}, 0) | ||
| err := EnsureVaultAccessForDev(lt, []string{"my-vault"}) | ||
| assert.Nil(t, err) | ||
| assert.Empty(t, client.Requests) | ||
| } | ||
|
|
||
| func TestEnsureVaultAccessForDevNoVaults(t *testing.T) { | ||
| target, client := createCloudTarget(t, io.Discard) | ||
| err := EnsureVaultAccessForDev(target, nil) | ||
| assert.Nil(t, err) | ||
| assert.Empty(t, client.Requests) | ||
| } | ||
|
|
||
| func TestEnsureVaultAccessForDevAlreadySet(t *testing.T) { | ||
| target, client := createCloudTarget(t, io.Discard) | ||
| // GET response: rule already present for t1.a1.i1 with "dev" context | ||
| existingRules := vaultResponse{ | ||
| Rules: []vaultAccessRule{ | ||
| {Application: "a1", Contexts: []string{"SANDBOX"}, ID: 0}, | ||
| }, | ||
| } | ||
| body, _ := json.Marshal(existingRules) | ||
| client.NextResponse(mock.HTTPResponse{Status: 200, Body: body}) | ||
|
|
||
| err := EnsureVaultAccessForDev(target, []string{"my-vault"}) | ||
| assert.Nil(t, err) | ||
| require.Len(t, client.Requests, 1) | ||
| assert.Equal(t, http.MethodGet, client.Requests[0].Method) | ||
| } | ||
|
|
||
| func TestEnsureVaultAccessForDevAddsRule(t *testing.T) { | ||
| target, client := createCloudTarget(t, io.Discard) | ||
| client.ReadBody = true | ||
|
|
||
| // GET: no existing rules | ||
| emptyRules := vaultResponse{Rules: []vaultAccessRule{}} | ||
| getBody, _ := json.Marshal(emptyRules) | ||
| client.NextResponse(mock.HTTPResponse{Status: 200, Body: getBody}) | ||
|
|
||
| // CSRF GET | ||
| csrfBody, _ := json.Marshal(map[string]string{"token": "test-csrf"}) | ||
| client.NextResponse(mock.HTTPResponse{Status: 200, Body: csrfBody}) | ||
|
|
||
| // PUT response with new rule confirmed | ||
| updatedRules := vaultResponse{ | ||
| Rules: []vaultAccessRule{ | ||
| {Application: "a1", Contexts: []string{"SANDBOX"}, ID: 0}, | ||
| }, | ||
| } | ||
| putBody, _ := json.Marshal(updatedRules) | ||
| client.NextResponse(mock.HTTPResponse{Status: 200, Body: putBody}) | ||
|
|
||
| err := EnsureVaultAccessForDev(target, []string{"my-vault"}) | ||
| assert.Nil(t, err) | ||
| require.Len(t, client.Requests, 3) | ||
| assert.Equal(t, http.MethodGet, client.Requests[0].Method) | ||
| assert.Equal(t, http.MethodGet, client.Requests[1].Method) // CSRF | ||
| assert.Equal(t, http.MethodPut, client.Requests[2].Method) | ||
| assert.Equal(t, "test-csrf", client.Requests[2].Header.Get("vespa-csrf-token")) | ||
| assert.Equal(t, "application/json", client.Requests[2].Header.Get("Content-Type")) | ||
| } | ||
|
|
||
| func TestEnsureVaultAccessForDevGetError(t *testing.T) { | ||
| target, client := createCloudTarget(t, io.Discard) | ||
| client.NextResponseError(io.EOF) | ||
| err := EnsureVaultAccessForDev(target, []string{"my-vault"}) | ||
| assert.NotNil(t, err) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SECRET_STORE_DEV_ALIAS is exported (all-caps identifier) but only used within this file. Consider making it unexported to avoid expanding the package’s public surface area unnecessarily (e.g., secretStoreDevAlias or similar).