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

tests/kola/ignition: add S3 ARN tests; test intra-cloud anonymous S3/GCS access #1717

Merged
merged 3 commits into from
May 18, 2022
Merged

tests/kola/ignition: add S3 ARN tests; test intra-cloud anonymous S3/GCS access #1717

merged 3 commits into from
May 18, 2022

Conversation

bgilbert
Copy link
Contributor

@bgilbert bgilbert commented May 6, 2022

Test the new ARN support in Ignition 2.14.0. S3 access points can't be accessed anonymously, so even the public objects need to be tested from inside an EC2 instance. Also test unauthenticated S3->EC2 and GCS->GCE fetches; the latter fails on Ignition < 2.14.0.

@dustymabe
Copy link
Member

dustymabe commented May 6, 2022

With these additions it feels a little like tests/kola/ignition/resource/s3 is misnamed. At first when looking I couldn't understand the real difference between the two tests.

The difference is that one of them is limited to just AWS (i.e. for testing authenticated access). Should we rename the test aws-authenticated-s3 (or anything else that's more explicit) to help explain? Or maybe at least add a description to the test and explanations for kola json values like:

# - platforms: qemu
# - This test should pass everywhere if it passes anywhere.
# - additionalDisks is only supported on qemu.
# - minMemory: 4096
# - Root reprovisioning requires at least 4GiB of memory.
# - additionalDisks: ["5G", "5G"]
# - Linear RAID is setup on these disks.
# - timeoutMin: 15
# - This test includes a lot of disk I/O and needs a higher
# timeout value than the default.

@bgilbert
Copy link
Contributor Author

bgilbert commented May 6, 2022

Yes, you're right, we should.

@bgilbert
Copy link
Contributor Author

bgilbert commented May 7, 2022

Updated!

@bgilbert
Copy link
Contributor Author

bgilbert commented May 7, 2022

Split out the two prep commits into #1723.

dustymabe
dustymabe previously approved these changes May 7, 2022
Copy link
Member

@dustymabe dustymabe left a comment

Choose a reason for hiding this comment

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

LGTM

dustymabe
dustymabe previously approved these changes May 7, 2022
@bgilbert bgilbert changed the title tests/kola/ignition: add S3 ARN tests tests/kola/ignition: add S3 ARN tests; test intra-cloud anonymous S3/GCS access May 10, 2022
@bgilbert bgilbert changed the title tests/kola/ignition: add S3 ARN tests; test intra-cloud anonymous S3/GCS access tests/kola/ignition: add S3 ARN and GCS tests; test intra-cloud anonymous S3/GCS access May 13, 2022
@bgilbert bgilbert changed the title tests/kola/ignition: add S3 ARN and GCS tests; test intra-cloud anonymous S3/GCS access tests/kola/ignition: add S3 ARN tests; test intra-cloud anonymous S3/GCS access May 13, 2022
@bgilbert bgilbert requested a review from dustymabe May 17, 2022 19:54
dustymabe
dustymabe previously approved these changes May 17, 2022
Copy link
Member

@dustymabe dustymabe left a comment

Choose a reason for hiding this comment

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

LGTM. A few more questions, though.

tests/kola/ignition/resource/authenticated-s3/config.bu Outdated Show resolved Hide resolved
tests/kola/ignition/resource/remote/test.sh Outdated Show resolved Hide resolved
Test the new ARN support in Ignition 2.14.0.

S3 access points can't be accessed anonymously, so even the public objects
need to be tested from inside an EC2 instance.
Have ext.config.ignition.resource.remote use the new noInstanceCreds flag
to disable instance credentials in EC2 and GCE instances, so the test
fetches resources anonymously.  Move the checks for authenticated access
of public objects into the authenticated-* tests.
@bgilbert bgilbert marked this pull request as ready for review May 17, 2022 21:57
@bgilbert
Copy link
Contributor Author

Updated!

Copy link
Member

@dustymabe dustymabe left a comment

Choose a reason for hiding this comment

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

LGTM

@bgilbert bgilbert merged commit 284c7f4 into coreos:testing-devel May 18, 2022
@bgilbert bgilbert deleted the arn/testing-devel branch May 18, 2022 04:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants