Clean old bundle data before new activation #4944
Conversation
| } | ||
| } | ||
|
|
||
| params.RootOverwrite = rootOverwrite |
There was a problem hiding this comment.
Other idea was to pass this in the context's value but did not seem right.
storage/disk/disk.go
Outdated
| if params.RootOverwrite { | ||
| newPath, ok := storage.ParsePathEscaped("/" + "") | ||
| if !ok { | ||
| return fmt.Errorf("storage path invalid: %v", newPath) | ||
| } | ||
|
|
||
| sTxn, err := db.doTruncateData(ctx, underlyingTxn, newDB, params, newPath, map[string]interface{}{}) | ||
| if err != nil { | ||
| return wrapError(err) | ||
| } | ||
|
|
||
| if sTxn != nil { | ||
| underlyingTxn = sTxn | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
This part I'm not sure if it wipes out everything. The tests seem to indicate that is not the case.
There was a problem hiding this comment.
Is this still an open issue with that change?
Isn't the same true when the roots differ? I.e. when the old roots are different from the new roots, does the old data get deleted properly? |
When the roots are specified in the manifest we delete the contents before calling the truncate call. https://github.com/open-policy-agent/opa/blob/main/bundle/store.go#L622. The issue comes up when no roots are defined. |
storage/disk/disk.go
Outdated
| @@ -263,6 +263,22 @@ func (db *Store) Truncate(ctx context.Context, txn storage.Transaction, params s | |||
| xid := atomic.AddUint64(&db.xid, uint64(1)) | |||
| underlyingTxn := newTransaction(xid, true, underlying, params.Context, db.pm, db.partitions, nil) | |||
|
|
|||
| if params.RootOverwrite { | |||
| newPath, ok := storage.ParsePathEscaped("/" + "") | |||
There was a problem hiding this comment.
💭 "/" + "" is "/", isn't it?
There was a problem hiding this comment.
You're right. Updated.
If OPA has an activated bundle that owns all roots and a new bundle with empty roots is to be activated, the old bundle's data should first be erased from the store. Currently both the old and new data is kept in the store. This commit attempts to fix this by providing an indication to the truncate call about the scenario in which the root is to be overwritten. Fixes: open-policy-agent#4940 Signed-off-by: Ashutosh Narkar <anarkar4387@gmail.com>
ashutosh-narkar
force-pushed
the
remove-old-data
branch
from
437d15f
to
503e2b5
Compare
If OPA has an activated bundle that owns all roots
and a new bundle with empty roots is to be activated, the
old bundle's data should first be erased from the store.
Currently both the old and new data is kept in the store.
This commit attempts to fix this by providing an indication to
the truncate call about the scenario in which the root is to be
overwritten.
Fixes: #4940
Signed-off-by: Ashutosh Narkar anarkar4387@gmail.com