Skip to content

feat(cli): add policy import/export commands to im-/export policies from/to json #4020

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.

Sign up for GitHub

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

Jump to bottom
Merged
jkowalski merged 22 commits into from
Oct 28, 2024
Merged

feat(cli): add policy import/export commands to im-/export policies from/to json #4020

jkowalski merged 22 commits into from
Oct 28, 2024

Conversation

@blenderfreaky
Copy link
Contributor

@blenderfreaky blenderfreaky commented Aug 2, 2024

This PR adds a policy import and policy export command to the cli.

The commands import and export policies from and to json files.

Syntax

$ kopia policy export [--json-indent] [--to-file=file] [--overwrite] [--global] [<target>]
$ kopia policy export --json-indent | tee policies.json
$ kopia policy export --to-file policies.json "foo@bar:/home/foobar"

Exports policies as json, optionally pretty-printed.
Prints to stdout if no file is specified.
Outputs all policies if --global and [<target>] are unspecified.

$ kopia policy import [--from-file=file]
$ cat policies.json | kopia policy import
$ kopia policy import --from-file policy.json

Imports policies from json, reads from stdin if no file is specified.

In-/Output is in the format

{
  "(global)": {
    "retention": {
      "keepLatest": 10,
      "keepHourly": 48,
      ...
    }
  },
  "foo@bar:/home/foobar": {
    ...
  }
}

Would close #2432 and #2785

@blenderfreaky blenderfreaky mentioned this pull request Aug 6, 2024
Closed
@tecosaur
Copy link

tecosaur commented Oct 25, 2024 •
edited
Loading

Bump @jkowalski @julio-lopez 🙂

I (and it seems a bunch of other people), are eagerly looking forward to this functionality in order to make process on automated configuration.

If you could find the time to take a look at this work, that would be brilliant and much appreciated.

(BTW, I think this PR also closes #4114)

@tecosaur tecosaur mentioned this pull request Oct 25, 2024
Closed
jkowalski
jkowalski reviewed Oct 27, 2024
View reviewed changes
jkowalski
jkowalski reviewed Oct 27, 2024
View reviewed changes
jkowalski
jkowalski reviewed Oct 27, 2024
View reviewed changes
jkowalski
jkowalski reviewed Oct 27, 2024
View reviewed changes
jkowalski
jkowalski reviewed Oct 27, 2024
View reviewed changes
jkowalski
jkowalski reviewed Oct 27, 2024
View reviewed changes
jkowalski
jkowalski reviewed Oct 27, 2024
View reviewed changes
jkowalski
jkowalski reviewed Oct 27, 2024
View reviewed changes
jkowalski
jkowalski reviewed Oct 27, 2024
View reviewed changes
@jkowalski
Copy link
Contributor

jkowalski commented Oct 27, 2024

This functionality looks great, can you add a quick e2e test and address the comments I've put in, then we can merge.

@codecov
Copy link

codecov bot commented Oct 27, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 82.69231% with 27 lines in your changes missing coverage. Please review.

Project coverage is 76.04%. Comparing base (cb455c6) to head (96c42a4).
Report is 316 commits behind head on master.

Files with missing lines Patch % Lines
cli/command_policy_import.go 77.21% 13 Missing and 5 partials ⚠️
cli/command_policy_export.go 88.00% 6 Missing and 3 partials ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #4020      +/-   ##
==========================================
+ Coverage   75.86%   76.04%   +0.17%     
==========================================
  Files         470      503      +33     
  Lines       37301    38709    +1408     
==========================================
+ Hits        28299    29435    +1136     
- Misses       7071     7312     +241     
- Partials     1931     1962      +31

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@blenderfreaky
Copy link
Contributor Author

blenderfreaky commented Oct 27, 2024

addressed most of your comments, gonna add tests and a remove/replace flag tomorrow

@blenderfreaky
Copy link
Contributor Author

blenderfreaky commented Oct 27, 2024

added tests and (hopefully) addressed all the comments

@jkowalski
Copy link
Contributor

jkowalski commented Oct 27, 2024

this is looking great, still have some linter errors, you can run make lint locally to see the linter errors.

@blenderfreaky
Copy link
Contributor Author

blenderfreaky commented Oct 27, 2024

should be fixed now

I used defer file.Close() //nolint:errcheck in two places, LMK if that's acceptable

@blenderfreaky
Copy link
Contributor Author

blenderfreaky commented Oct 27, 2024

it fails on windows only
I'm on linux so this may be tricky to debug

I assume it's due to the policy test target being specified as "user@host:" + path with the path returned by testutil using backslashes. If I'm right, this should fix it.

@blenderfreaky
Copy link
Contributor Author

blenderfreaky commented Oct 27, 2024

gonna try to address the code coverage as well

@blenderfreaky
Copy link
Contributor Author

blenderfreaky commented Oct 28, 2024 •
edited
Loading

should be good now (hopefully)

afaict more test coverage requires triggering internal kopia errors

jkowalski
jkowalski approved these changes Oct 28, 2024
View reviewed changes
Copy link
Contributor

@jkowalski jkowalski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for the contribution!

@blenderfreaky
Copy link
Contributor Author

blenderfreaky commented Oct 28, 2024

The windows test still seem to be failing, seems to be related to the paths

Gonna try to spin up a VM to debug

@jkowalski
Copy link
Contributor

jkowalski commented Oct 28, 2024

policies in Windows use backslashes for path names jarek@windowsbox:C:\Users\Jarek\somedir

@blenderfreaky
Copy link
Contributor Author

blenderfreaky commented Oct 28, 2024

it failed without filepath.ToSlash as well, so that can't be the only issue

@jkowalski
Copy link
Contributor

jkowalski commented Oct 28, 2024

I have a Windows dev box, let me try it...

@jkowalski
Copy link
Contributor

jkowalski commented Oct 28, 2024

removing ToSlash fixed the issue on Windows.

@blenderfreaky
Copy link
Contributor Author

blenderfreaky commented Oct 28, 2024

Interesting that the two ToSlash calls in the import tests don't break anything

@jkowalski
Copy link
Contributor

jkowalski commented Oct 28, 2024 •
edited
Loading

i think slash-separated policies will be imported/exported just fine, but will not really take effect on Windows boxes (since it uses backslashes). We should fix that perhaps at some point.

@jkowalski jkowalski enabled auto-merge (squash) October 28, 2024 01:32
@jkowalski jkowalski merged commit a9e178e into kopia:master Oct 28, 2024
@tecosaur
Copy link

tecosaur commented Oct 28, 2024

It's brilliant to see this happen, thanks for getting this done blenderfreaky and Jarek!

alvistar pushed a commit to alvistar/kopia that referenced this pull request Oct 29, 2024
@blenderfreaky @jkowalski @alvistar
feat(cli): add policy import/export commands to im-/export policies f…
ed16303 
…rom/to json (kopia#4020)

* feat(cli): add policy export command

* feat(cli): add policy import command

* chore(cli): remove unused policyTargetFlags in commandPolicyImport

* feat(cli): allow limiting which policies to import

* docs(cli): document policy import and export commands

* feat(cli): add policy import/export commands

* fix(cli): apply suggestions

* fix(general): unintentional commit

* feat(cli: add policy export tests

* feat(cli): add policy import tests

* feat(cli): add deleteOtherPolicies option for policy import

* docs(cli): stdin/stdout in policy import/export and --delete-other-policies flag

* chore(cli): fix linter issues

* fix(cli): fix newly introduced errors

* fix(cli): fix failing windows tests

* chore(cli): more test coverage for policy import/export

* fixed windows test

---------

Co-authored-by: Jarek Kowalski <[email protected]>
mcamou pushed a commit to mcamou/kopia that referenced this pull request Oct 30, 2024
@blenderfreaky @jkowalski @mcamou
feat(cli): add policy import/export commands to im-/export policies f…
5027091 
…rom/to json (kopia#4020)

* feat(cli): add policy export command

* feat(cli): add policy import command

* chore(cli): remove unused policyTargetFlags in commandPolicyImport

* feat(cli): allow limiting which policies to import

* docs(cli): document policy import and export commands

* feat(cli): add policy import/export commands

* fix(cli): apply suggestions

* fix(general): unintentional commit

* feat(cli: add policy export tests

* feat(cli): add policy import tests

* feat(cli): add deleteOtherPolicies option for policy import

* docs(cli): stdin/stdout in policy import/export and --delete-other-policies flag

* chore(cli): fix linter issues

* fix(cli): fix newly introduced errors

* fix(cli): fix failing windows tests

* chore(cli): more test coverage for policy import/export

* fixed windows test

---------

Co-authored-by: Jarek Kowalski <[email protected]>
@julio-lopez
Copy link
Collaborator

julio-lopez commented Nov 20, 2024

@julio-lopez julio-lopez mentioned this pull request Nov 20, 2024
Closed
@taltamir taltamir mentioned this pull request Jan 9, 2025
Closed
@julio-lopez julio-lopez mentioned this pull request Mar 30, 2025
Merged
julio-lopez added a commit that referenced this pull request Mar 30, 2025
@julio-lopez
fix(cli): add newline after policy export output (#4476)
8f32be2 
- #4341
- #4020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jkowalski jkowalski jkowalski approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Declarative configuration

4 participants

@blenderfreaky @tecosaur @jkowalski @julio-lopez