WIP: feat(ske) access ressource#1437
Conversation
|
This PR was marked as stale after 7 days of inactivity and will be closed after another 7 days of further inactivity. If this PR should be kept open, just add a comment, remove the stale label or push new commits to it. |
|
This PR was closed automatically because it has been stalled for 7 days with no activity. Feel free to re-open it at any time. |
|
@Manuelvaas I think this PR could solve #1360 |
| if cl.Access == nil { | ||
| m.Access = defaultAccess | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Is this still the workaround? I would guess that if nothing is returned by the API, we should save nothing into the state.
There was a problem hiding this comment.
I've double checked the API spec, it's still marked as optional. Also there are some tests in ske_test.go that use mocked cluster responses. So I went for the fallback to the default objects here.
But technically this should be unreachable code besides tests (which could be adapted)
| var diags diag.Diagnostics | ||
|
|
||
| var idpObject basetypes.ObjectValue | ||
| if cl.Access.Idp == nil { |
There was a problem hiding this comment.
same as above
| Description: descriptions["access_idp_type"], | ||
| Required: true, | ||
| Validators: []validator.String{ | ||
| stringvalidator.OneOf("stackit"), |
There was a problem hiding this comment.
I'll have to check if we can validate that here or if we should just put it into the description.
Co-authored-by: Manuel Vaas <34416897+Manuelvaas@users.noreply.github.com>
Merging this branch will increase overall coverage
Coverage by fileChanged files (no unit tests)
Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code. Changed unit test files
|
| Description: descriptions["access"], | ||
| Optional: true, | ||
| Computed: true, | ||
| Default: objectdefault.StaticValue(defaultAccess), |
There was a problem hiding this comment.
| Default: objectdefault.StaticValue(defaultAccess), | |
I don't see why we would want a default on TF side here. The API got fixed and shouldn't require any special handling. This also applies to the logic in the mapFields function, from my understanding no special handling is needed there.
4 participants
Description
relates to #1234
Checklist
make fmtexamples/directory)make generate-docs(will be checked by CI)make test(will be checked by CI)make lint(will be checked by CI)