-
Notifications
You must be signed in to change notification settings - Fork 95
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 support for opa-versions and sentinel-versions admin Api #758
Conversation
6b19643
to
a9566d0
Compare
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.
Awesome work. There are a couple of blockers down below. Also when you create a new resource, make sure to generate the mocks by calling ./generate_mocks.sh
.
oList, err := client.Admin.OpaVersions.List(ctx, nil) | ||
require.NoError(t, err) | ||
|
||
assert.NotEmpty(t, oList.Items) |
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 this assertion always be true for any instance? We create a fresh instance for our nightly TFE tests. Normally, for _List
tests I like to create a few resources beforehand to ensure than something is returned by the List() call.
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.
Please correct me if I am wrong, but I thought these tool versions are prepopulated in the test instances from this script here: https://github.com/hashicorp/atlas/blob/f016d274ae6bb7d4da54fdeede86a794125dbaac/lib/data/tool_versions.yml#L1028
Because it looks like admin_terraform _versions tests do the same thing.
@sebasslash Thanks for the review 🙏 but I am going to convert this into a draft PR as we realised we cannot publish the docs until we have Pinned Policy Runtime Version released. I will move it into review when we have the docs in prod! :) Thanks again! |
2247dae
to
9cd353e
Compare
This is ready to be reviewed :) |
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.
Can you also add these to generate_mocks.sh
as well as to the "Admin" section of the API Coverage in the README?
1558275
to
ef3d4ef
Compare
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.
Ran tests without issue. go-tfe API looks good.
ef3d4ef
to
2223803
Compare
Reminder to the contributor that merged this PR: if your changes have added important functionality or fixed a relevant bug, open a follow-up PR to update CHANGELOG.md with a note on your changes. |
Description
Add support for the new
opa-versions
and thesentinel-versions
admin API which will be required for creating the opa-version and the sentinel-version providersTesting plan
Integration tests (Results below)
Output from tests