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.

Sign up for GitHub

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

Jump to bottom

New data sources #1816

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

New data sources #1816

wants to merge 3 commits into from

Conversation

steveAG
Copy link
Contributor

@steveAG steveAG commented Nov 15, 2023

Sorry for the multiple resource PR, I caught that in the contribution guidelines when creating the PR, I'll keep that in mind for future contributions, and I can cherry pick these into three PRs if you prefer.

Adds the following data sources:

duytiennguyen-okta
duytiennguyen-okta reviewed Nov 17, 2023
View reviewed changes
okta/data_source_okta_apps.go
}
_ = d.Set("apps", appsArr)
} else {
_ = d.Set("apps", make([]map[string]interface{}, 0))
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this error out when it cannot filter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When no match is found based on the filter?

I don't think so, data sources that return a single instance of a resource throw an error on no match, but this is consistent with the behavior of all of the existing multi-instance data sources I've seen in the provider.
ie users, groups, & group assignments all return an empty array if there are no matches.

duytiennguyen-okta
duytiennguyen-okta requested changes Nov 17, 2023
View reviewed changes
Copy link
Contributor

@duytiennguyen-okta duytiennguyen-okta left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I don't have the issue with the purpose of the PR itself, however as we are trying to move away from plugin sdk to framework sdk, can you please port it over to framework sdk? Here is an example of another contributor who has done it #1726 . Thank you for your effort @steveAG

@monde
Copy link
Collaborator

monde commented Nov 28, 2023

@steveAG I'm going to help here and bring your original work into a PR that I can refactor to use the TF Plugin Framework. Do you have Okta Individual Contributor License Agreement on file with us https://developer.okta.com/cla/ ?

@monde
Copy link
Collaborator

monde commented Nov 28, 2023

Okta internal reference https://oktainc.atlassian.net/browse/OKTA-671288

@steveAG
Copy link
Contributor Author

steveAG commented Nov 29, 2023

Thanks a million @monde!

I've been inundated with work and haven't gotten to digging into moving to the framework SDK yet.

Yes I have an agreement on file, signed and sent in June 2023.

@monde monde added the triaged Triaged into internal Jira label Nov 29, 2023
@exitcode0 exitcode0 mentioned this pull request Apr 15, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@duytiennguyen-okta duytiennguyen-okta duytiennguyen-okta requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
triaged Triaged into internal Jira
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Feature Request: okta_apps data source Feature Request: okta_admin_role_custom data source
3 participants
@steveAG @monde @duytiennguyen-okta