-
Notifications
You must be signed in to change notification settings - Fork 8
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
Implement cache import/export (includes auto-generated config files) #17
Conversation
- Remove all the options except for mode switches (and config file path) - Change logging so it goes either to the console or to a file, not both. - Remove the notion of a "template" configuration, going with defaults instead that are always read and then overwritten by the configuration. - Auto-create a config file if there isn't one on first run. - Get rid of all the optionals in settings, since the config always has a value. - Break out the "cache control" command into separate commands, so that only one can be invoked at a time. - Make settings be serializable so that it can be exported to a config. - Start moving all modules to "eyre" reports for better clarity about problems.
This adds interviewing, so it fixes #13 completely. This also does `eyre`-handling of secure proxy setup.
I made a change to the rustfmt configuration file but forgot to reformat all the project files after doing so.
I forgot that clippy's checks are more thorough than the compiler's... and some of these were significant!!
This fixes #9. It also fixes a bug in the general refactoring of main done earlier in this PR: I had left out settings validation and log initialization for the cache control commands.
To do this required: - separating storing responses from processing them, and only processing them when operating live or importing forwarded data. - making all the cache operations take a specified database, rather than always using the cache database. This meant making most of the operations standalone rather than methods on the cache. - looking for matched pairs of request/response when importing from a forwarded database. This change also turns off all statement logging in sqlx unless the `FRL_PROXY_ENABLE_STATEMENT_LOGGING` environment variable is defined.
* Update the version in Cargo.toml * Update all the dependencies to current versions * Integrate transactionality into the cache (fixes #16) * Update the changelog.md and latest.md files * Make sure the database gets closed at the end of each run.
@adorton-adobe This PR supersedes the other two I had opened waiting for review. It includes all of their changes, so I am going to close them. Just focus the review on this one, which hopefully takes us all the way to v1.0. |
Sounds good. I'll take a look. Once we get this merged, I will do a 1.0.0rc1 release while I work on updating the docs. |
The config init workflow breaks when I use the shift key (e.g. when typing Not sure what we can do until that crate gets an update. I'll be sure to mention this issue in the docs. It should be fine since |
I am also happy to update the host parsing to understand any delimiter between the host and port (e.g., space, hyphen, or other non-shifted delimiters :). We could also specify host and port in separate config entries, which actually would allow to (for example) specify one port for use with SSL and another for use with non-SSL which would help make the |
Sounds good |
This is also a factor in other options (file paths, remote host), but I don't think that will be too much of an issue. |
Here is the cert file needed for testing: frl-proxy.pkcs.zip. |
I notice that console-rs/console#83 has a comment that says this problem is due to the |
After I
|
I just checked the code and we definitely close the database we're importing from. But I am noticing the same behavior. Here is the reference article from SQLite about those files. Having read it very carefully, and followed this link, I now believe that those files are left because I am opening the import database in read-only mode. I have made the change to open the import database read-write and that does fix the problem. |
Testing reveals that the import command is leaving SQLite temp files for that database created on the disk. After reading [reference article from SQLite](https://sqlite.org/tempfiles.html) about those files, and followed [this link](https://sqlite.org/wal.html), I concluded that those files are left because I am opening the import database in read-only mode. So I am now opening the import database read-write and that seems to fix the problem. While testing I found that I had removed a needed `wrap-err` explanation for when the cache database is missing but required. So I put that back in.
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.
Looks great - I just have a few suggested tweaks.
One other thing to note is when testing cached deactivation, import/export reported importing/exporting 4 requests. 3 of those are my prior activations (Photoshop, Bridge, Acrobat). Should those have been cleared from the cache when I activated those licenses? The rest of the cached deactivation workflow worked as expected - those licenses are inactive.
Terrific, thanks @adorton-adobe. I understand and am great with all your suggestions and will implement them today or tomorrow. WRT your question about the number of request/response pairs: what you saw is expected and correct. When operating in forward mode, the proxy replays all the stored requests/responses in order and stores the results without processing them. Then, during the import of the forwarding database, the results are "replayed" as if they had just come in to a live proxy. So first the two activation requests are processed and stored (waiting for the clients to come back and find them) and then the deactivation response is processed: it is this processing which removes the already received activation responses (and doesn't store the deactivation response because those are never replayed to clients). So once the import has finished, any clients who come back and request an activation response will not find one, so the client side matches the server side. |
@adorton-adobe On the older PR you mentioned that you wanted to change the configuration file suffix back to |
I think it's totally fine to keep the |
RIght, I understood why you wanted to change it to toml, and I guess I'm saying I think that really applies to all of the filenames: I think it's way better to use file extensions to reflect the content type of the file and embed the function of the file in the name. Also, the reason to take out the
and it looks great. I'd like to check it in, if you're OK with that. |
Looks good. Once you push those changes, I think we can merge. |
1. Rename default config, cache, cert, and log files to have their names show their function and their extensions show their content type. Use default names that start with `proxy-` rather than `frl-proxy-` so they sort together and aren't confusable with the executable file name. 2. Default to using a log file rather than logging to the console. 3. Provide default prompts in the configuration process so a user can review their settings by invoking `configure` and then just hitting carriage return everywhere. 4. Extend signal handling to capture SIGTERM as well as SIGINT so most shutdowns are graceful. 5. Make forwarding more interactive: have it print the number of requests it's going to process, and the number of success/failure responses it receives. This is now ready to be merged.
OK, changes pushed. Please approve and merge once it finishes building. |
Summary
eyre
for error reportingTesting Steps for auto-creation of config files (#13)
Testing steps for import/export and external forwarding (#10)
https://frl-proxy.brotsky.net:8843
as its proxy address127.0.0.1:8443
with SSL and add an entry to your hosts file mapping 127.0.01 tofrl-proxy.brotsky.net
. Use the attachedpkcs
certificate file (passwordtest
) for your testing. Use the defaults for proxy database file (frl-proxy.cache
) and proxy log file (frl-proxy.log
), and set the log destination tofile
. Use any logging level you want.store
mode withfrl-proxy start --mode store
frl-proxy export frl-proxy.forward
. It will report exporting 2 requests.frl-proxy.cache
file tofrl-proxy.stored
frl-proxy.forward
file tofrl-proxy.cache
forward
mode withfrl-proxy start --mode forward
. It will automatically stop.frl-proxy.cache
file tofrl-proxy.forward
frl-proxy.stored
file tofrl-proxy.cache
frl-proxy import frl-proxy.forward
. It will report importing 2 request/response pairs.store
mode withfrl-proxy start --mode store
.frl-proxy.forward
file.adobe-licensing-toolkit -l
to verify your activated licenses. The output should look something like this, with theNpdId
coming from the license file of your installation:store
mode withfrl-proxy start --mode store
adobe-licensing-toolkit -t -n your-npdId-from-above
. It will fail.frl-proxy export frl-proxy.forward
. It will report exporting 1 request.frl-proxy.cache
file tofrl-proxy.stored
frl-proxy.forward
file tofrl-proxy.cache
forward
mode withfrl-proxy start --mode forward
. It will automatically stop.frl-proxy.cache
file tofrl-proxy.forward
frl-proxy.stored
file tofrl-proxy.cache
frl-proxy import frl-proxy.forward
. It will report importing 1 request/response pair.store
mode withfrl-proxy start --mode store
.NOTE: Once you have successfully activated Photoshop, it's not quite enough to deactivate it to repeat the tests. You also have to remove the cached license from the credential store. The license will have a key that is a base 64 encoded form of the app ID and the certificate generation it uses. You can use the adobe-license-decoder utility to show you what that base64 ID is, because it's the first part of the OperatingConfig name of the license file.
Related Issues and PRs
Fixes #9
Fixes #10
Fixes #12
Fixes #13
Fixes #16
Supersedes #14
Supersedes #15