v3.4.3 Broken due to dependancy Update

[speatzle]

It seems that v3.4.3 (and probably most other releases) are broken.
The Release fails to build because of a breaking change in https://github.com/Azure/go-ntlmssp (v0.0.0-20220621081337-cb9428e4ac1e)
They Merged this PR Azure/go-ntlmssp#32 in Azure/go-ntlmssp@cb9428e

That PR changed the function signature of ntlmssp.ProcessChallenge to have an additional bool causing the following build failure:

# github.com/go-ldap/ldap/v3
../../go/pkg/mod/github.com/go-ldap/ldap/[email protected]/bind.go:500:96: not enough arguments in call to ntlmssp.ProcessChallenge
	have ([]byte, string, string)
	want ([]byte, string, string, bool)

https://github.com/Azure/go-ntlmssp does apparently not tag releases. Which is why it gets upgraded even tho it had a breaking change

[iamcaje-psh]

https://github.com/Azure/go-ntlmssp does apparently not tag releases. Which is why it gets upgraded even tho it had a breaking change

I had to downgrade to :

go get github.com/Azure/[email protected]

to get my project to build. Added a comment to my go.mod letting me know not to upgrade:

require (
	// this version works - newer commits have breaking changes - do not update
	// github.com/Azure/go-ntlmssp v0.0.0-20211209120228-48547f28849e // indirect
	github.com/Azure/go-ntlmssp v0.0.0-20211209120228-48547f28849e // indirect
...
)

[cpuschma]

Thank you for opening an issue @speatzle. Did you run go get -u ./... beforehand? The working commit hash is referenced in our go.mod files:

ldap/go.mod

Lines 1 to 9 in a3dcdda

	module github.com/go-ldap/ldap
	go 1.14
	require (
	github.com/Azure/go-ntlmssp v0.0.0-20211209120228-48547f28849e
	github.com/go-asn1-ber/asn1-ber v1.5.4
	golang.org/x/crypto v0.0.0-20220331220935-ae2d96664a29 // indirect
	)

The build only failed when I manually updated the dependencies:

C:\Users\nevo\IdeaProjects\go-ldap>go get -v -u ./...
go: downloading github.com/Azure/go-ntlmssp v0.0.0-20220621081337-cb9428e4ac1e
go: downloading golang.org/x/crypto v0.0.0-20220525230936-793ad666bf5e
go: upgraded github.com/Azure/go-ntlmssp v0.0.0-20211209120228-48547f28849e => v0.0.0-20220621081337-cb9428e4ac1e
go: upgraded golang.org/x/crypto v0.0.0-20220331220935-ae2d96664a29 => v0.0.0-20220525230936-793ad666bf5e

C:\Users\nevo\IdeaProjects\go-ldap>go test -v ./...  
# github.com/go-ldap/ldap [github.com/go-ldap/ldap.test]
.\bind.go:521:96: not enough arguments in call to ntlmssp.ProcessChallenge
        have ([]byte, string, string)
        want ([]byte, string, string, bool)
FAIL    github.com/go-ldap/ldap [build failed]

Nevertheless, I hope the devs at Azure/go-ntlmssp take care of it. In case if they don't revert the change and all new releases stop working and to save others the frustration, we can add the dependencies with a working version to the repository and fix the latest release.. but older releases would still be affected when trying to update the dependencies.

If you're having trouble building your project: Ensure your go.mod file hasn't been updated and still references to the working version:

github.com/Azure/go-ntlmssp v0.0.0-20211209120228-48547f28849e

If this is not the case, try the following steps to get it working again:

1. Revert the go.mod + go.sum file to use correct / working commit
2. Run go clean -modcache to clear your mod cache just in case
3. In your project, download your dependencies again using go mod download

C:\Users\nevo\IdeaProjects\go-ldap>git checkout HEAD -- go.mod go.sum

C:\Users\nevo\IdeaProjects\go-ldap>go clean -modcache

C:\Users\nevo\IdeaProjects\go-ldap>go mod download

C:\Users\nevo\IdeaProjects\go-ldap>go test ./...
ok      ok      github.com/go-ldap/ldap 9.452s

[speatzle]

Our CI Pipeline automatically ran and did the go get -u and failed the step so it's fine for now.

[tirupatibg]

Hi @speatzle Could we fix this? based on the requirements asked by go-ntlmssp. I think adding new bool param is just a simple change here right?

[speatzle]

IMO https://github.com/Azure/go-ntlmssp should revert Azure/go-ntlmssp#32
and then tag v1.0.0
after that they could merge Azure/go-ntlmssp#32 again and tag v2.0.0

this would ensure that all old releases of go-ldap continue to work and would allow for breaking changes down the line without affecting go-ldap and other downstream projects.

go-ldap could then adopt the new function signature of Azure/go-ntlmssp#32 in a new release

[cpuschma]

To clarify for both this issue and the PR over at Azure/go-ntlmssp#34, already released versions of go-ldap are still working unless you manually force an dependency update using go get -u. If Azure/go-ntlmssp do not revert their breaking change then we have no other choice but to:

1. Update our functions to support the new function signature
2. Maybe start vendoring our dependencies to avoid further issues
3. Add a note to our README to inform users about potential problems when using older versions

[abhinavdeep01]

For those not able to build in CI Pipeline add a flag -skipDepUpdate=true
This worked for me.

[Justin-W]

I had to downgrade to :

Thanks to @speatzle and @iamcaje-psh for identifying the root cause of the problem. Instead of downgrading using @iamcaje-psh 's workaround, I added the following to my go.mod file, to prevent the broken Azure/go-ntlmssp API from getting upgraded even if/when you run go get -u ./... afterwards.

// github.com/Azure/[email protected] introduced backwards-incompatible API changes
// SEE: https://github.com/Azure/go-ntlmssp/issues/33
// SEE: https://github.com/go-ldap/ldap/issues/381
replace (
	// NOTE: If this module adds any additional broken versions (without reverting the API breaking change), the aggressive replace will be needed.
	// However, if they fix the API in the next version, we can remove the aggressive replace and keep only the narrow replace (just in case).
	github.com/Azure/go-ntlmssp => github.com/Azure/go-ntlmssp v0.0.0-20211209120228-48547f28849e // aggressive: replaces EVERY version with the LKG (backwards-compatible) version
	github.com/Azure/go-ntlmssp v0.0.0-20220621081337-cb9428e4ac1e => github.com/Azure/go-ntlmssp v0.0.0-20211209120228-48547f28849e // narrow: replaces only the 1 currently-known-broken version
)

If Azure/go-ntlmssp#34 gets merged as the next version, then the aggressive replace will become unnecessary. Or if https://github.com/go-ldap/ldap is instead updated to use the new https://github.com/Azure/go-ntlmssp API, then the replace directives can be removed to re-enable normal dependency upgrades.

[cpuschma]

If Azure/go-ntlmssp#34 gets merged as the next version, then the aggressive replace will become unnecessary. Or if https://github.com/go-ldap/ldap is instead updated to use the new https://github.com/Azure/go-ntlmssp API, then the replace directives can be removed to re-enable normal dependency upgrades.

It is still unclear whether the PR at Azure/go-ntlmssp will be merged or not. I say we give them a week to decide, otherwise we might opt for vendoring or adopting their breaking change.