-
Notifications
You must be signed in to change notification settings - Fork 48
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 vTPM test #1011
base: master
Are you sure you want to change the base?
Add vTPM test #1011
Conversation
94d8727
to
b7039c7
Compare
I'll fix the yetus errors and squash at the end. |
826d976
to
cc8039d
Compare
Running it locally with secrets available works fine and test pass, and when I run it using
but here it time out because the app is not available ?! |
c48ff14
to
9132daf
Compare
Also there is this PR #1008 which bumps EVE version to 13.0.0, can we bump EVE version to 13.0.0 in your PR as well? |
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.
Do I get it right, that the scripts we have are the ones running inside VMs?
go.mod
Outdated
go 1.20 | ||
go 1.22 | ||
|
||
toolchain go1.22.5 |
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.
why do we need toolchain?
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.
Ahhhh, I was to late to comment on toolchain :DD
tests/aziot/aziot_test.go
Outdated
// The ID Scope is required to configure azure-iot in the VM, | ||
// we can get it from the Azure IoT Hub -> Device Provisioning Service -> Overview | ||
// and copy the "ID Scope". | ||
aziotIdScope = os.Getenv("AZIOT_ID_SCOPE") |
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.
Yetus complaint to rename variable :)
tests/aziot/aziot_test.go
Outdated
}() | ||
|
||
// wait for the deployed app to appear in the list | ||
time.Sleep(30 * time.Second) |
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.
wasn't there a way to read app status directly from controller?
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.
yes, but in the first few seconds it return "app not found", even though it is deployed.
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.
see a few lines down :
err = eveNode.AppWaitForRunningState(appName, 60*5)
if err != nil {
t.Fatalf("Failed to wait for app to start: %v", err)
}
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.
huh, so that's a bug and a behaviour we don't want in Eden, I'll create a ticket for it then. Because it shouldn't be like that, right? :D
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.
I think so, let me check again and make sure it is happening consistently or no.
tests/aziot/aziotenroll.go
Outdated
} | ||
|
||
func readPublicKey(handle tpmutil.Handle) ([]byte, error) { | ||
// unfortunaly we can't used SWTPM socket directly, it is blocked becuse |
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.
Minor yetus changes
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.
will fix all/squash after the review is completed.
yes, the scripts are copied to vm to set up the azure-iot-edge and run the services, then I check if everything is running without error. |
tests/aziot/aziot_test.go
Outdated
} | ||
|
||
// execute the script to create the necessary TPM keys | ||
_, err = eveNode.AppSSHExec(appName, createKeyScriptPath) |
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.
side note: I'd abstract path to a file into a reader for AppSSHExec function, it'll be more flexible to call and if you want to run some local examples (unit tests for a function) you can create readers in code :)
Edit: I meant AppSCPCopy, SSHExec runs command AFAIK
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.
you mean change AppSCPCopy to something like AppSCPCopy(strAppname, ioReader, strRemoteFile) ?
but the internal scp call needs a path.
a358341
to
ac86cf9
Compare
4f46f8a
to
6a6e8e8
Compare
6a6e8e8
to
63dc2a3
Compare
This is a test for EVE PR #4071. It test the vTPM feature of EVE and aziot on EVE both legacy (using EVE-TOOLS) and latest (using vTPM). Signed-off-by: Shahriyar Jalayeri <[email protected]>
@yash-zededa the variables are still not accessible in the test, your help is much appreciated.
|
63dc2a3
to
279f8aa
Compare
PR's won't have access to secrets. Unless WF uses |
You mean there is no way to test this is working without merging? |
Unfortunately, yes, this behavior is intentional from GitHub. If you want to test pull requests with logins, the workflow needs to be updated to trigger using However, this approach allows anyone to modify the workflow and potentially access the secrets, so it's safer to prevent pull requests from triggering if there are any changes to the workflow. |
This tests lf-edge/eve#4071
/cc @eriknordmark