-
Notifications
You must be signed in to change notification settings - Fork 72
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
Additional group attributes #137
base: main
Are you sure you want to change the base?
Additional group attributes #137
Conversation
…so fix some bugs related to JSON parsing
I updated the merge request to resolve the conflicts that came up after #133 was merged (renamed distinguished_name to dn). I also updated the description of the merge request to reflect that it still fixes an issue with the distinguished_name, where it doesn't properly update when the group is renamed or moved. |
Has anyone had a chance to review this yet? For what it's worth, we've been using the provider with these changes in production for about half a year now, but it would be nice to use the official build again. Also, I have some more minor changes lined up that would be based on these changes and I'm waiting for this to be merged before opening another PR. |
Can this be reviewed? This is the only thing blocking us from using this provider. |
I think this stops local winrm working, I get this error on a compiled edition of the PR;
In the existing provider I can do;
|
@eperdeme Hm... I would not expect the provider to work without any of those configurations. How or where would it connect? If you use environment variables, sure. But explicitly setting all of those values to an empty string seems to me like it shouldn't work. Maybe I'm missing something? We use my branch in production that I have released here: https://registry.terraform.io/providers/hanneshayashi/ad/latest We configure the provider for Kerberos (that PR was also from me) like this: provider "ad" {
winrm_hostname = "<our DC>"
winrm_username = "<our service account>"
winrm_password = ""
winrm_port = 5986
winrm_proto = "https"
winrm_insecure = true
krb_realm = "<our domain>"
krb_conf = "/etc/krb5.conf"
krb_keytab = "<our keytab>"
} If you try my release, be sure to specify the source in your required_providers like this: terraform {
required_providers {
ad = {
source = "hanneshayashi/ad"
}
}
} |
@hanneshayashi the provider supports local execution as per https://github.com/hashicorp/terraform-provider-ad/blob/main/docs/index.md#note-about-local-execution-windows-only This method occurs when for example you run ci runners on a Windows server and bind that runner to the svc amount. This is how we currently run this provider. |
* Tidyup (hashicorp#142) - Generate docs using latest tfplugindocs tool - Update plugin SDK - Use go 1.18 * fix typo in docs * Update Changelog * update go-version * v0.4.4 * Migrate Provider Releases from TeamCity to GitHub Actions Internal RFC References: * ENGSRV-035 * ENGSRV-064 * SEC-036 * SEC-061 * TF-279 _Please note: This process can be adjusted to suit your needs, but it will require changes to the workflow setup._ The initial release workflow submitted here is triggered by pushing a semantic version tag prepended with a `v` to the repository. For example: ```shell git switch main # or your release branch git pull git tag v1.2.3 git push origin v1.2.3 ``` The most important distinction from the TeamCity release process is that the repository must be fully prepared for the release, including the `CHANGELOG.md` file. Providers can decide the most appropriate process to manage the CHANGELOG or any release notes as part of this new process. This initial workflow will automatically grab contents from the top of the CHANGELOG through the previous release tag header line. [Example provider CHANGELOG](https://github.com/hashicorp/terraform-provider-tls/blob/156ae39c7e55ee8597f859a77ae2db739527376b/CHANGELOG.md) and its [GitHub Release description](https://github.com/hashicorp/terraform-provider-tls/releases/tag/v3.3.0). Please reach out if you have questions. * Onboard project to Releases API * Update to include terraform registry manifest Co-authored-by: Kyriakos Oikonomakos <[email protected]> Co-authored-by: Kyriakos Oikonomakos <[email protected]> Co-authored-by: tf-release-bot <[email protected]> Co-authored-by: Brian Flad <[email protected]> Co-authored-by: Jeanne Franco <[email protected]> Co-authored-by: claire labry <[email protected]>
This reverts commit 5bfc51f.
merge main
@eperdeme I see. I must admit, that is not a setup I have personally tried. I just did a merge from main into this branch to make sure that it is still up to date. There were a lot of changes to the vendored code, but nothing that should have any impact on authentication. Can you please check again? Also can you confirm that the behaviour you are experiencing does not happen with the latest version of the official provider? As far as I know, the last change to the authentication was done when the Kerberos code was merged and I just want to make sure that didn't break anything :) |
Is there a timeline on when this might be merged and released? |
Really in need of this, could it be merged please? |
Hi, anybody worked on this yet? Is it viable? What is missing to complete it? |
Hey Guys, any updates on this ? :) |
I just resolved a minor merge conflict that seems to have popped up at some point. I don't know why the triage step is failing but it seems to only be relevant for the PR labels and the unit tests still seem to pass. |
Thanks a lot @hanneshayashi ! Turning now to @manicminer and @koikonom who initially reviewed the linked issue #91 and apparently worked on implementation of the same for |
Description
This pull request adds the following new features:
It should also fix some issues that I have encountered with the provider:
"foo" = ""bar""
)I also did some slight refactoring to enable code reuse. None of the changes should be breaking.
I think there is still some room for improvement, but I didn't want to make any overly broad changes to the code base. Although I did add a new package to the vendor directory for the customDiff. It's part of the already used helper package, so it should hopefully be fine.
References
Community Note