Skip to content

Commit

Permalink
fix(secrets): move secrets out-of-band from dataset
Browse files Browse the repository at this point in the history 
this came from work on another PR, but secrets should be completely separate from datasets, and need to be plumbed through the call stack as such

closes #609
  • Loading branch information
@b5
b5 committed Nov 14, 2018
1 parent a9b9eb5 commit a258a23
Show file tree
Hide file tree
Showing 9 changed files with 62 additions and 164 deletions.
6 changes: 3 additions & 3 deletions actions/actions_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func addCitiesDataset(t *testing.T, node *p2p.QriNode) repo.DatasetRef {
dsp.Name = tc.Name
dsp.BodyBytes = tc.Body

ref, _, err := SaveDataset(node, dsp, false, true)
ref, _, err := SaveDataset(node, dsp, nil, false, true)
if err != nil {
t.Fatal(err.Error())
}
Expand All @@ -108,7 +108,7 @@ func addFlourinatedCompoundsDataset(t *testing.T, node *p2p.QriNode) repo.Datase
dsp.Name = tc.Name
dsp.BodyBytes = tc.Body

ref, _, err := SaveDataset(node, dsp, false, true)
ref, _, err := SaveDataset(node, dsp, nil, false, true)
if err != nil {
t.Fatal(err.Error())
}
Expand All @@ -124,7 +124,7 @@ func addNowTransformDataset(t *testing.T, node *p2p.QriNode) repo.DatasetRef {
dsp.Name = tc.Name
dsp.Transform.ScriptPath = "testdata/now_tf/transform.star"

ref, _, err := SaveDataset(node, dsp, false, true)
ref, _, err := SaveDataset(node, dsp, nil, false, true)
if err != nil {
t.Fatal(err.Error())
}
Expand Down
14 changes: 4 additions & 10 deletions actions/dataset.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (
)

// SaveDataset initializes a dataset from a dataset pointer and data file
func SaveDataset(node *p2p.QriNode, changesPod *dataset.DatasetPod, dryRun, pin bool) (ref repo.DatasetRef, body cafs.File, err error) {
func SaveDataset(node *p2p.QriNode, changesPod *dataset.DatasetPod, secrets map[string]string, dryRun, pin bool) (ref repo.DatasetRef, body cafs.File, err error) {
var (
prev *dataset.Dataset
prevPath string
Expand Down Expand Up @@ -77,13 +77,10 @@ func SaveDataset(node *p2p.QriNode, changesPod *dataset.DatasetPod, dryRun, pin
// TODO - consider making this a standard method on dataset.Transform
script := cafs.NewMemfileReader(changes.Transform.ScriptPath, changes.Transform.Script)

var secrets map[string]string
var config map[string]interface{}
if changesPod.Transform == nil {
secrets = make(map[string]string)
config = make(map[string]interface{})
} else {
secrets = changesPod.Transform.Secrets
config = changesPod.Transform.Config
}
bodyFile, err = ExecTransform(node, prev, script, bodyFile, secrets, config, mutateCheck)
Expand Down Expand Up @@ -128,7 +125,7 @@ func clearPaths(ds *dataset.Dataset) {

// UpdateDataset brings a reference to the latest version, syncing over p2p if the reference is
// in a peer's namespace, re-running a transform if the reference is owned by this profile
func UpdateDataset(node *p2p.QriNode, ref *repo.DatasetRef, dryRun, pin bool) (res repo.DatasetRef, body cafs.File, err error) {
func UpdateDataset(node *p2p.QriNode, ref *repo.DatasetRef, secrets map[string]string, dryRun, pin bool) (res repo.DatasetRef, body cafs.File, err error) {
if dryRun {
node.LocalStreams.Print("🏃🏽‍♀️ dry run\n")
}
Expand Down Expand Up @@ -159,10 +156,10 @@ func UpdateDataset(node *p2p.QriNode, ref *repo.DatasetRef, dryRun, pin bool) (r
return
}

return localUpdate(node, ref, dryRun, pin)
return localUpdate(node, ref, secrets, dryRun, pin)
}

func localUpdate(node *p2p.QriNode, ref *repo.DatasetRef, dryRun, pin bool) (res repo.DatasetRef, body cafs.File, err error) {
func localUpdate(node *p2p.QriNode, ref *repo.DatasetRef, secrets map[string]string, dryRun, pin bool) (res repo.DatasetRef, body cafs.File, err error) {
var (
bodyFile cafs.File
commit = &dataset.CommitPod{}
Expand Down Expand Up @@ -203,13 +200,10 @@ func localUpdate(node *p2p.QriNode, ref *repo.DatasetRef, dryRun, pin bool) (res

node.LocalStreams.Print("🤖 executing transform\n")

var secrets map[string]string
var config map[string]interface{}
if ref.Dataset.Transform == nil {
secrets = make(map[string]string)
config = make(map[string]interface{})
} else {
secrets = ref.Dataset.Transform.Secrets
config = ref.Dataset.Transform.Config
}
bodyFile, err = ExecTransform(node, ds, script, bodyFile, secrets, config, nil)
Expand Down
136 changes: 15 additions & 121 deletions actions/dataset_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,15 @@ func TestUpdateDatasetLocal(t *testing.T) {
cities := addCitiesDataset(t, node)

expect := "transform script is required to automate updates to your own datasets"
if _, _, err := UpdateDataset(node, &cities, false, true); err == nil {
if _, _, err := UpdateDataset(node, &cities, nil, false, true); err == nil {
t.Error("expected update without transform to error")
} else if err.Error() != expect {
t.Errorf("error mismatch. %s != %s", expect, err.Error())
}

now := addNowTransformDataset(t, node)
prevPath := now.Path
now, _, err := UpdateDataset(node, &now, false, false)
now, _, err := UpdateDataset(node, &now, nil, false, false)
if err != nil {
t.Error(err)
}
Expand Down Expand Up @@ -58,12 +58,12 @@ func TestUpdateDatasetRemote(t *testing.T) {
}

// run a local update to advance history
now0, _, err := UpdateDataset(peers[0], &now, false, false)
now0, _, err := UpdateDataset(peers[0], &now, nil, false, false)
if err != nil {
t.Error(err)
}

now1, _, err := UpdateDataset(peers[1], &now, false, false)
now1, _, err := UpdateDataset(peers[1], &now, nil, false, false)
if err != nil {
t.Error(err)
}
Expand Down Expand Up @@ -126,7 +126,7 @@ func TestSaveDataset(t *testing.T) {
BodyBytes: []byte("[]"),
}

ref, _, err := SaveDataset(n, ds, true, false)
ref, _, err := SaveDataset(n, ds, nil, true, false)
if err != nil {
t.Errorf("dry run error: %s", err.Error())
}
Expand All @@ -147,94 +147,13 @@ func TestSaveDataset(t *testing.T) {
Structure: &dataset.StructurePod{Format: dataset.JSONDataFormat.String(), Schema: map[string]interface{}{"type": "array"}},
BodyBytes: []byte("[]"),
}
ref, _, err = SaveDataset(n, ds, false, true)
ref, _, err = SaveDataset(n, ds, nil, false, true)
if err != nil {
t.Error(err)
}

ds = &dataset.DatasetPod{
Peername: ref.Peername,
Name: ref.Name,
Commit: &dataset.CommitPod{
Title: "add transform script",
Message: "adding an append-only transform script",
},
Transform: &dataset.TransformPod{
Syntax: "starlark",
ScriptBytes: []byte(`def transform(ds,ctx):
bd = ds.get_body()
bd.append("hey")
ds.set_body(bd)`),
},
}
ref, _, err = SaveDataset(n, ds, false, true)
if err != nil {
t.Fatal(err)
}

// save new manual changes
ds = &dataset.DatasetPod{
Peername: ref.Peername,
Name: ref.Name,
Commit: &dataset.CommitPod{
Title: "update meta",
Message: "manual change that'll negate previous transform",
},
Meta: &dataset.Meta{
Title: "updated title",
Description: "updated description",
},
}
ref, _, err = SaveDataset(n, ds, false, true)
if err != nil {
t.Error(err)
}
if ref.Dataset.Transform != nil {
t.Error("expected manual save to remove transform")
}

// recall previous transform
tfds, err := Recall(n, "tf", ref)
if err != nil {
t.Error(err)
}

ds = &dataset.DatasetPod{
Peername: ref.Peername,
Name: ref.Name,
Commit: &dataset.CommitPod{
Title: "re-run transform",
Message: "recall transform & re-run it",
},
Transform: tfds.Transform,
}
ref, _, err = SaveDataset(n, ds, false, true)
if err != nil {
t.Error(err)
}
if ref.Dataset.Transform == nil {
t.Error("expected recalled transform to be present")
}
}

func TestSaveDatsetTfConfigSecrets(t *testing.T) {
n := newTestNode(t)

ds := &dataset.DatasetPod{
Name: "test_tf_with_config_and_secrets",
Commit: &dataset.CommitPod{
Title: "initial commit",
Message: "manually create a baseline dataset",
},
Meta: &dataset.Meta{
Title: "another test dataset",
},
Structure: &dataset.StructurePod{Format: dataset.JSONDataFormat.String(), Schema: map[string]interface{}{"type": "array"}},
BodyBytes: []byte("[]"),
}
ref, _, err := SaveDataset(n, ds, false, true)
if err != nil {
t.Error(err)
secrets := map[string]string{
"bar": "secret",
}

ds = &dataset.DatasetPod{
Expand All @@ -249,18 +168,15 @@ func TestSaveDatsetTfConfigSecrets(t *testing.T) {
Config: map[string]interface{}{
"foo": "config",
},
Secrets: map[string]string{
"bar": "secret",
},
ScriptBytes: []byte(`def transform(ds,ctx):
ScriptBytes: []byte(`def transform(ds,ctx):
ctx.get_config("foo")
ctx.get_secret("bar")
bd = ds.get_body()
bd.append("hey")
ds.set_body(bd)`),
},
}
ref, _, err = SaveDataset(n, ds, false, true)
ref, _, err = SaveDataset(n, ds, secrets, false, true)
if err != nil {
t.Fatal(err)
}
Expand All @@ -277,31 +193,13 @@ func TestSaveDatsetTfConfigSecrets(t *testing.T) {
Title: "updated title",
Description: "updated description",
},
Transform: &dataset.TransformPod{
// TODO - the fact that we need to completely re-supply the transform function
// here is a bug. There's currently no way to supply secrets to a transform without
// recalling the previous version. Because of this, I think secrets need to be adjusted
// to be completely out-of-band to datasets, and instead plumbed down from a new field
// lib.SaveParams.Secrets. The Secrets field on dataset.DatasetPod should be removed
Syntax: "starlark",
Config: map[string]interface{}{
"foo": "config",
},
Secrets: map[string]string{
"bar": "secret",
},
ScriptBytes: []byte(`def transform(ds,ctx):
ctx.get_config("foo")
ctx.get_secret("bar")
bd = ds.get_body()
bd.append("hey")
ds.set_body(bd)`),
},
}
ref, _, err = SaveDataset(n, ds, false, true)
ref, _, err = SaveDataset(n, ds, nil, false, true)
if err != nil {
t.Error(err)

}
if ref.Dataset.Transform != nil {
t.Error("expected manual save to remove transform")
}

// recall previous transform
Expand All @@ -310,10 +208,6 @@ func TestSaveDatsetTfConfigSecrets(t *testing.T) {
t.Error(err)
}

tfds.Transform.Secrets = map[string]string{
"bar": "secret",
}

ds = &dataset.DatasetPod{
Peername: ref.Peername,
Name: ref.Name,
Expand All @@ -323,7 +217,7 @@ func TestSaveDatsetTfConfigSecrets(t *testing.T) {
},
Transform: tfds.Transform,
}
ref, _, err = SaveDataset(n, ds, false, true)
ref, _, err = SaveDataset(n, ds, secrets, false, true)
if err != nil {
t.Error(err)
}
Expand Down
2 changes: 1 addition & 1 deletion actions/registry_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func TestRegistry(t *testing.T) {
dsp.Name = tc.Name
dsp.BodyBytes = tc.Body

ref, _, err := SaveDataset(node, dsp, false, true)
ref, _, err := SaveDataset(node, dsp, nil, false, true)
if err != nil {
t.Fatal(err.Error())

Expand Down
11 changes: 11 additions & 0 deletions api/datasets.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,17 @@ func (h *DatasetHandlers) saveHandler(w http.ResponseWriter, r *http.Request) {
ReturnBody: r.FormValue("return_body") == "true",
}

if r.FormValue("secrets") != "" {
p.Secrets = map[string]string{}
if err := json.Unmarshal([]byte(r.FormValue("secrets")), &p.Secrets); err != nil {
util.WriteErrResponse(w, http.StatusBadRequest, fmt.Errorf("parsing secrets: %s", err))
return
}
} else if dsp.Transform != nil && dsp.Transform.Secrets != nil {
// TODO remove this, require API consumers to send secrets separately
p.Secrets = dsp.Transform.Secrets
}

if err := h.Save(p, res); err != nil {
util.WriteErrResponse(w, http.StatusInternalServerError, err)
return
Expand Down
22 changes: 10 additions & 12 deletions cmd/save.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,29 +135,27 @@ func (o *SaveOptions) Run() (err error) {
},
}

p := &lib.SaveParams{
Dataset: dsp,
DatasetPath: o.FilePath,
Private: false,
Publish: o.Publish,
DryRun: o.DryRun,
Recall: o.Recall,
}

if o.Secrets != nil {
if !confirm(o.Out, o.In, `
Warning: You are providing secrets to a dataset transformation.
Never provide secrets to a transformation you do not trust.
continue?`, true) {
return
}

dsp.Transform = &dataset.TransformPod{}
if dsp.Transform.Secrets, err = parseSecrets(o.Secrets...); err != nil {
if p.Secrets, err = parseSecrets(o.Secrets...); err != nil {
return err
}
}

p := &lib.SaveParams{
Dataset: dsp,
DatasetPath: o.FilePath,
Private: false,
Publish: o.Publish,
DryRun: o.DryRun,
Recall: o.Recall,
}

res := &repo.DatasetRef{}
if err = o.DatasetRequests.Save(p, res); err != nil {
return err
Expand Down
Loading

0 comments on commit a258a23

Please sign in to comment.