use Secrets for all API keys

[dobbs]

Feature Description

There are several places where the operator needs API keys in order to work correctly. Many of those keys want to be protected as kubernetes Secrets.

Examples include:
webhook auth_password
PagerDuty service_key
VictorOps key
OpsGenie api_key

[mhaddon]

It would also be nice if it was not needed to be specified on every alert as-well.

Maybe there should be a different operator per api-key, and I can keep that information restricted to the namespace that the newrelic operator is running in.
Then when I want to create an alert I tell it which operator it should use.

Or a single operator, but it references different secrets or configurations stored in the operators namespace. (A similar setup to cert-manager).

I would rather not have my API-Keys in every namespace all over the place.

I do see in the spec though: APIKeySecret

newrelic-kubernetes-operator/api/v1/alerts_policy_types.go

Line 37 in adf063e

APIKeySecret NewRelicAPIKeySecret `json:"api_key_secret,omitempty"`

So I will try that.

[douglascamata]

I don't like the idea of linking the operator directly with an api key. This makes the scenario were a cluster has applications reporting to different accounts, each one with their own alerts, very complicated and over-engineered. All the alert policies are from the same type, so there would need to be logic in each operator to know which alert policies objects each one own, etc.

Each alert policy belongs to an account, that is linked to it through the api key. It makes total sense that each alert policy references an api key.

[mhaddon]

Its more to do with centralising the configuration.

Like with external-dns I do not have to define the configuration and permissions on each lb in each namespace, with cert-manager I do not have to define the configuration and permissions in each namespace.

Just seems a bit weird that I have to define this information on every alert policy.

[douglascamata]

These other softwares you mentioned work in a model that is very different from New Relic alerts and also aren't operators, @mhaddon.

You define this information on every alert policy because there are real world scenarios where each alert policy could be linked to a different account and deploying the operator N times in this case would be extremely weird + requires specific code in the operator to handle it.

There are other options that could be less disruptive. For example, maybe the operator could be configured with a default api key and put it on every alert policy object that doesn't already define one.

[mhaddon]

The multiple operator idea was just one of the two things I mentioned in the first post to just spit-ball ideas on how it can be achieved. The approach that cert-manager has with Cluster-Issuers is quite nice, in my opinion, as you can define them centrally and just refer to those configurations.

You make some good points, I do think though it would be nice to find a solution where it is not needed to be configured every time. However maybe I am just thinking about the whole thing incorrectly.
It could also have the region and account_id configured with it.

It is like you are separating the context from the actual alerts configuration. I think it makes sense.

apiVersion: nr.k8s.newrelic.com/v1
kind: AlertContext
metadata:
  name: my-context
spec:
  account_id: <your New Relic account ID>
  api_key: <your New Relic personal API key>
  api_key_secret: "my-newrelic-secret"
  region: "US"

apiVersion: nr.k8s.newrelic.com/v1
kind: AlertsPolicy
metadata:
  name: my-policy
spec:
  context: "my-context"
  ....

The AlertContext's could be provisioned in the same namespace as the operator and then the policies can be provisioned, wherever.

Possibly if no "context" is defined in the spec for AlertsPolicy, then it will default to look for an AlertContext called "default".