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

[zitadel/terraform-provider-zitadel#205] Add idp_oidc #206

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[zitadel/terraform-provider-zitadel#205] Add idp_oidc #206

wants to merge 2 commits into from

Conversation

Rumbles
Copy link

@Rumbles Rumbles commented Dec 3, 2024 •
edited by stebenz
Loading

Definition of Ready

  • Add idp_oidc resource to provider.
  • Links to Missing: zitadel_idp_oidc #205
  • Acceptance criteria are met
  • All open todos and follow ups are defined in a new ticket and justified
  • Deviations from the acceptance criteria and design are agreed with the PO and documented.
  • No debug or dead code
  • My code has no repetitions
  • All non-functional requirements are met
  • The generic lifecycle acceptance test passes for affected resources.
  • Examples are up-to-date and meaningful. The provider version is incremented.
  • Docs are generated.
  • Code is generated where possible.

Closes #205

@Rumbles Rumbles force-pushed the 205_oidc_idp branch 8 times, most recently from e758040 to f072978 Compare December 3, 2024 13:25
@muhlemmer muhlemmer requested a review from stebenz December 9, 2024 10:30
stebenz
stebenz requested changes Dec 9, 2024
View reviewed changes
zitadel/provider.go Show resolved Hide resolved
stebenz
stebenz requested changes Dec 16, 2024
View reviewed changes
Copy link
Contributor

@stebenz stebenz 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.

Tested the functionality, everything seems to be in order.
Could you please also regenerate the docs that the datasource is also included?
Thank you for all your effort.

templates/data-sources/idp_oidc.md.tmpl
@@ -0,0 +1,16 @@
---
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like this template is not yet generated for the docs.

stebenz
stebenz requested changes Dec 16, 2024
View reviewed changes
zitadel/idp_oidc/resource.go
return &schema.Resource{
Description: "Resource representing an OIDC IDP on the instance.",
Schema: map[string]*schema.Schema{
helper.OrgIDVar: helper.OrgIDResourceField,
Copy link
Contributor

Choose a reason for hiding this comment

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

idp_oidc does not need an OrgID as it is an instance level resource, please have a look at the other like idp_oauth resource as a reference.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is also the reason the testing fails in the pipeline.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stebenz stebenz stebenz requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
os-contribution resources
Projects
Product Management
Status: 👀 In review
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Missing: zitadel_idp_oidc
3 participants
@Rumbles @stebenz @hifabienne