Skip to content
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

Add vault variable example #2241

Merged
merged 10 commits into from
Jan 24, 2024

Conversation

tpmccallum
Copy link
Contributor

@tpmccallum tpmccallum commented Jan 24, 2024

Adding vault variable example, so that we can link to it from the documentation Re: this developer documentation issue

This example is meant to go hand-in-hand with the https://developer.fermyon.com/spin/v2/dynamic-configuration#vault-application-variable-provider-example

Signed-off-by: tpmccallum <tim.mccallum@fermyon.com>
@tpmccallum tpmccallum requested review from itowlson and rylev January 24, 2024 02:59
@tpmccallum tpmccallum self-assigned this Jan 24, 2024
@tpmccallum tpmccallum added the documentation Improvements or additions to documentation label Jan 24, 2024
@tpmccallum
Copy link
Contributor Author

Builds and runs:

$ spin build
Building component vault-variable-test with `cargo build --target wasm32-wasi --release`
    Finished release [optimized] target(s) in 0.06s
Finished building all Spin components
$ spin up --runtime-config-file runtime_config.toml
Logging component stdio to ".spin/logs/"

Serving http://127.0.0.1:3000
Available Routes:
  vault-variable-test: http://127.0.0.1:3000 (wildcard)

Making the request does make an error, though:

thread '<unnamed>' panicked at 'could not get variable: Error::Undefined("no variable for \"vault-variable-test\".\"test_password\"")', src/lib.rs:11:52
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
2024-01-24T03:03:17.158681Z ERROR spin_trigger_http: Error processing request: guest invocation failed

Caused by:
    0: error while executing at wasm backtrace:
           0: 0x15676 - <unknown>!__rust_start_panic
           1: 0x15431 - <unknown>!rust_panic
           2: 0x1540c - <unknown>!std::panicking::rust_panic_with_hook::h59179097e3fe00d4
           3: 0x14772 - <unknown>!std::panicking::begin_panic_handler::{{closure}}::h685c1054195e4d4b
           4: 0x1469f - <unknown>!std::sys_common::backtrace::__rust_end_short_backtrace::hd312084468c4357c
           5: 0x14d22 - <unknown>!rust_begin_unwind
           6: 0x22cb6 - <unknown>!core::panicking::panic_fmt::h826facefad16f5a7
           7: 0x24fc8 - <unknown>!core::result::unwrap_failed::hf48c8310675f070e
           8: 0x560f - <unknown>!vault_variable_test::handle_vault_variable_test::hd63e5b2c7245f8f7
           9: 0x3bb6 - <unknown>!spin_sdk::http::executor::run::hd6b683a35367e806
          10: 0x5901 - <unknown>!wasi:http/incoming-handler@0.2.0-rc-2023-10-18#handle
       note: using the `WASMTIME_BACKTRACE_DETAILS=1` environment variable may show more debugging information
    1: wasm trap: wasm `unreachable` instruction executed    

Copy link
Collaborator

@itowlson itowlson left a comment

Choose a reason for hiding this comment

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

The content is fine, just a few possible issues (the key one being the variables in spin.toml), sorry


[dependencies]
anyhow = "1"
spin-sdk = { git = "https://github.com/fermyon/spin", tag = "v2.1.0" }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
spin-sdk = { git = "https://github.com/fermyon/spin", tag = "v2.1.0" }
spin-sdk = { path = "../../sdk/rust" }

We don't want to have to keep the version in sync grin

@@ -0,0 +1,19 @@
spin_manifest_version = 2
Copy link
Collaborator

Choose a reason for hiding this comment

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

I might be missing something but the manifest doesn't seem to define the variables?

#[http_component]
fn handle_vault_variable_test(req: Request) -> Result<impl IntoResponse> {
let password = std::str::from_utf8(req.body().as_ref()).unwrap();
let expected = variables::get("test_password").expect("could not get variable");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Worth commenting this line to draw attention to it?

@@ -0,0 +1,5 @@
[[config_provider]]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it worth adding a comment to this file? Or talk about it in a README? Not sure, just a vague wonder...

Signed-off-by: tpmccallum <tim.mccallum@fermyon.com>
Signed-off-by: tpmccallum <tim.mccallum@fermyon.com>
Signed-off-by: tpmccallum <tim.mccallum@fermyon.com>
Signed-off-by: tpmccallum <tim.mccallum@fermyon.com>
Copy link
Collaborator

@itowlson itowlson left a comment

Choose a reason for hiding this comment

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

there is one change needed but otherwise all good - thanks!

6. Test the application:

```bash
curl -i http://127.0.0.1:3000
Copy link
Collaborator

Choose a reason for hiding this comment

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

needs moar request body

Suggested change
curl -i http://127.0.0.1:3000
curl --data guess http://127.0.0.1:3000

Copy link
Collaborator

Choose a reason for hiding this comment

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

oh this might need a docs update too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great catch @itowlson
Updated in last commit of the PR, thanks again !!!

Signed-off-by: tpmccallum tim.mccallum@fermyon.com

Signed-off-by: Timothy McCallum <tim.mccallum@fermyon.com>
@tpmccallum tpmccallum enabled auto-merge January 24, 2024 04:32
tpmccallum added a commit to fermyon/developer that referenced this pull request Jan 24, 2024
Signed-off-by: tpmccallum tim.mccallum@fermyon.com

This PR will only pass the tests once the spinframework/spin#2241 is merged and the example app URL is live.

Signed-off-by: Timothy McCallum <tim.mccallum@fermyon.com>
tpmccallum and others added 4 commits January 25, 2024 05:56
Signed-off-by: tpmccallum tim.mccallum@fermyon.com

Co-authored-by: Ryan Levick <me@ryanlevick.com>
Signed-off-by: Timothy McCallum <tim.mccallum@fermyon.com>
Signed-off-by: tpmccallum tim.mccallum@fermyon.com

Co-authored-by: Ryan Levick <me@ryanlevick.com>
Signed-off-by: Timothy McCallum <tim.mccallum@fermyon.com>
Signed-off-by: tpmccallum tim.mccallum@fermyon.com

Co-authored-by: Ryan Levick <me@ryanlevick.com>
Signed-off-by: Timothy McCallum <tim.mccallum@fermyon.com>
Signed-off-by: tpmccallum tim.mccallum@fermyon.com

Signed-off-by: Timothy McCallum <tim.mccallum@fermyon.com>
@tpmccallum tpmccallum merged commit 99c219f into spinframework:main Jan 24, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants