chore: 'omitempty' to Oauth2 fields with type Secret to avoid requiring them

[JorTurFer]

After the PR supporting the RFC7523, there are 2 fields with Secret type inside the Oauth2 struct. This isn't a problem at all, but there are some yaml marshalers in other projects using this pkg (I've discovered this bumping the dep in otel-collector) and they can fail during the marshaling process because one (or more of them) can be empty and it's trying to execute custom marshal logic.

Probably the best option should be handling the case everywhere where custom logic is applied, but it's also true that adding omitempty tag in the source solves it for any usecase.

I've tested this change directly there and it works perfectly

[bwplotka]

Why not set omitempty to all fields?

We got hit by this too in the past when operating things with different versions (operator vs operated thing) using Prometheus config with a new field without omitempty.

[JorTurFer]

Why not set omitempty to all fields?

Do you mean here or there? Gotcha, you meant here xD

[bwplotka]

Everywhere.

At some point I was even adding global options to some YAML projects: goccy/go-yaml#691 ;p

Is there a disadvantage? We never assume something because we don't see the field vs default value (: (we have healthy defaults)

[bwplotka]

Anyway, let's merge this now .

[JorTurFer]

I was updating the branch.. xD Maybe another a follow-up PR or just if someone comes again and cry like me xD

[JorTurFer]

Could I ask for a new release? (if it's not planned already). I'll take the new version and update my PRs on otel-collector and prometheus

[bwplotka]

follow PR is great, sorry for merging too quickly!

[bwplotka]

Should we add this follow up first, before release?

[JorTurFer]

Should we add this follow up first, before release?

Sure! I'm going to prepare it in a couple of minutes and we can ship both together actually

[JorTurFer]

well... All the other Secret properties had omitempty set, only the OAuth2 ones didn't have it (sorry because I didn't see 😢 )

Should I add it for string fields too? Maybe it's overkill? If the problem is the parser, probably yes because strict parsing which checks all the fields can fail even for empty strings