feat: support for Ory Network

[adamstrawson]

Adds support for Ory Network by adding a new api key flag.

When specified, the Authorization header is included in all requests.

Related Issue or Design Document

#132

Checklist

• I have read the contributing guidelines and signed the CLA.
• I have referenced an issue containing the design document if my change introduces a new feature.
• I have read the security policy.
• I confirm that this pull request does not address a security vulnerability.
  If this pull request addresses a security vulnerability,
  I confirm that I got approval (please contact [email protected]) from the maintainers to push the changes.
• I have added tests that prove my fix is effective or that my feature works.
• I have added the necessary documentation within the code base (if appropriate).

Further comments

[Demonsthere]

Hello there!
Really happy to see the interest here :)
Imho we cannot implementing the apiKey in such a way (plaintext value in the CR), rather use a secretReference and either mount or read the supplied secret, similar to secretName: my-secret-123

[adamstrawson]

Hello there! Really happy to see the interest here :) Imho we cannot implementing the apiKey in such a way (plaintext value in the CR), rather use a secretReference and either mount or read the supplied secret, similar to secretName: my-secret-123

I understand the concerns, we use Flux HelmReleases, so it's easy for us to inject these as secrets still. So two options come to mind if you have a preference?

1. Rather than using a flag, use an environment variable instead, and within the Helm Chart have a value to define the secret

{{- if .Values.apiKeySecret }}
    env:
    - name: HYDRA_API_KEY
      valueFrom:
        secretKeyRef:
          name: {{ .Values.apiKeySecret }}
{{- end }}

2. Similar to what you said, using a flag to set the secretName and then using the Kubernetes client to fetch the secret value.

[Demonsthere]

I think we can connect both approaches :)
1 - it is good for a global apiKey, which is then used by other resources.
2 - we can define CR level options like

apiKeySecretRef:
  name: foo

which is optional, and if not supplied we default to the secret in 1, if that is not defines too, don't use apikey altogether

[adamstrawson]

Disclaimer: I've only just recently starting picking up Go, so fairly new to it still - any feedback is appreciated!

This now supports both a global environment variable, or a CR level option.

Option 1: Environment Variable

If HYDRA_API_KEY is set, Authorization will be appended to all requests.

Open to suggestions on a more appropriate name for this variable too.

Option 2: CR Option

This will also replace any value defined in the global HYDRA_API_KEY environment variable.

spec:
  hydraAdmin:
    url: <ory_network_url>
    apiKeySecretRef:
      name: hydra-secret
      key: api-key # Optional
      namespace: auth # Optional

I'll leave the PR in draft for any feedback.

[Demonsthere]

Hi there! Sorry for the delay, but I was off for some time. The PR is coming along nicely, leaving some feedback regarding code structure :)

[Demonsthere]

Suggested change

	Name string `json:"name,omitempty"`
	Name string `json:"name"`

The whole object can be optional, but once given the name of the secret must be supplied.
Moreover, for the object to properly validated by k8s we need to add the annotations like here

	// +kubebuilder:validation:Type=object
	// +nullable
	// +optional
	//
	// Metadata is abritrary data
	Metadata apiextensionsv1.JSON `json:"metadata,omitempty"`

[Demonsthere]

Did you forget about the Key here ;d?

[Demonsthere]

for consistency please move the default key value to consts and refer to it here :)

[Demonsthere]

The default value of the field can be defined in 2 ways:
1 - in the code using a const like we do for other variables
2 - define it in the CRD using // +kubebuilder:default=foo

[Demonsthere]

Since the name should be always given, this shouldn't be required, or throw an error if the name is empty

[Demonsthere]

I think this would make more sense as a method on the OAuth2Client type

[Demonsthere]

Not a fan of this pattern. I think we should define the clients either in the New function, or pass them to the function to avoid the possibility of creating multiple instances. Then we can refactor this function into a method and create some unit tests :)

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

---

adamstrawson seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}