Remove Hashing From Service Endpoints

[davidcorrigan714]

All Submissions:

• Have you added an explanation of what your changes do and why you'd like us to include them?
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.
• My code follows the code style of this project.
• I ran lint checks locally prior to submission.
• Have you checked to ensure there aren't other open PRs for the same update/change?

What about the current behavior has changed?

This PR removes hashing of sensitive data from all the service connection resources to resolve #719 #692 #719 #732 #629 . While I applaud the effort to protect sensitive data, it was very buggy and I'm not at all confident all those issues are resolvable while maintaining the hashing. Hopefully Hashicorp has a standard solution at some point, see this issue. I also reviewed the handling of all service connection fields marked as sensitive in ADO to make sure they're never read back into the state file.

Note that this may unmask sensitive data currently stored in state by service connection resources - though when cleaning this up it was more inconsistently applied than I thought. I can find no mention in the documentation that this provider attempts to protect sensitive data, though many providers have an explicit warning that they will store sensitive data in plain text in state files. Example. Hashicorp provides guidance to users to secure state so I'm 50/50 on whether the warning text should be added to every service connection doc page.

Note that acceptance tests cannot currently cover all the instances of bugs this fixes. I wrote a custom task extension to write sensitive service connection out to pipeline logs in plain text to debug and validate a lot of the updates. It'd be nice to incorporate that into the acceptance tests, but that's quite the undertaking and the current ADO Go API may not even have all the necessary hooks, on top of needing to publish an extension to retrieve the protected data.

Does this introduce a change to go.mod, go.sum or vendor/?

• Yes
• No

Does this introduce a breaking change?

• Yes
• No

[davidcorrigan714]

Well I ran the linting and formatting checks, can't say the output was exactly clean. gofmt apparently doesn't play nice with windows line endings and the linter is complaining about vendoring which I don't think I changed with this.

[xuzhang3]

@davidcorrigan714 would you mind if I directly push changes to this PR to fix the CI errors?

[xuzhang3]

Well I ran the linting and formatting checks, can't say the output was exactly clean. gofmt apparently doesn't play nice with windows line endings and the linter is complaining about vendoring which I don't think I changed with this.

I normally set git config core.autocrlf=true to ensure line ending will automatically formatted across platforms.

[davidcorrigan714]

Have at it, going to be a little while before I have time to do any updates myself.

…

On Tue, Jun 6, 2023 at 10:14 PM xuzhang3 ***@***.***> wrote: @davidcorrigan714 <https://github.com/davidcorrigan714> would you mind if I directly push changes to this PR to fix the CI errors? — Reply to this email directly, view it on GitHub <#743 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ALOFOJXA7KHWIP4DI6PLCDDXJ7W2TANCNFSM6AAAAAAWO2XNK4> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[xuzhang3]

Can you give me the push permissions? I cannot push to this PR.

[davidcorrigan714]

Apparently I don't have that option since my PR is from an organization account. Not sure what the cleanest way is to get it out of that account, doesn't look easy to just give you access to that specific branch on our fork either.

[davidcorrigan714]

Suppose I can fork the fork into my personal account and submit a new PR. It's not like there's been much happening on this one. Or you're free to sync it and do a new PR from another account, doesn't bother me at all if this code ends up officially coming from my account or a PR directly from me.

[xuzhang3]

@davidcorrigan714 closing this PR as the changes has been merged by #856