Add access monitoring rules cache and tctl interactions

[EdwardDowling]

This Pr registers the access monitoring rules service, adds it to the cache, and adds the tctl resource commands for the new resource.

part of 3123

changelog: Add ability to manage access monitoring rules via tctl

[github-actions]

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

[marcoandredinis]

Can we also please add tests to tctl?
It seems we don't support tctl create -f operation (aka upsert)

[github-actions]

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

[github-actions]

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

[github-actions]

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

[github-actions]

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

[github-actions]

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

[github-actions]

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

[github-actions]

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

[kimlisa]

i was trying to test this out in my web UI i'm currently working on, and noticed something odd. when i create, the created resource does not show up when i try to list, unless i restart teleport? 🤔

[EdwardDowling]

i was trying to test this out in my web UI i'm currently working on, and noticed something odd. when i create, the created resource does not show up when i try to list, unless i restart teleport? 🤔

The cache must not be set up properly, will fix this now and add some more tests.

[EdwardDowling]

i was trying to test this out in my web UI i'm currently working on, and noticed something odd. when i create, the created resource does not show up when i try to list, unless i restart teleport? 🤔

@kimlisa
I've been trying to replicate this and cant seem to get it to happen.
Can you let me know what the logs are for you when the list doesn't work for you?

[EdwardDowling]

@rosstimothy Can't seem to figure out the issue mentioned above. It doesn't happen when creating and listing via tctl or when creating via tctl and listing from the ui changes mentioned. Only when both creating and listing from the ui. Can you take a look so we know if these changes are safe to merge or if that bug is due to these?

[kimlisa]

i have a update:

in the web UI, if i create yaml, the cache registers it (just like tctl) so listing works and I noticed this difference:

this is the resource that gets returned when i create through the web UI yaml editor, note that kind is defined, that is because in the yaml editor it is pre-defined:

metadata:
  name: new_rule_name2
  namespace: ""
  description: ""
  labels: {}
  expires: null
  id: 0
  revision: 50c5d8de-6b4c-4088-8471-248212bf70e7
kind: access_monitoring_rule     <--------------------- defined
subkind: ""
version: v7
spec:
  subjects:
  - access_request
  states: []
  condition: hoco poco loco doco choco pam choco
  notification:
    name: slack
    recipients:
    - poco
    - loco

this is the resource that is not created with yaml and that does not get registered in the cache (unless you restart), note that i can create a resource without a name and leave field kind and version to be empty.

metadata:
  name: ""
  namespace: ""
  description: ""
  labels: {}
  expires: null
  id: 0
  revision: ef975e30-e7de-4c82-aba2-dfc51e2810ca
kind: ""     <---------------------- probably why it does not get registered in cache
subkind: ""
version: ""
spec:
  subjects:
  - access_request
  states: []
  condition: sdfasdfsdf
  notification:
    name: slack
    recipients: []

sure I can put these validation in the client, but shouldn't we be checking them in the auth layer (as like a central place to check for errors)? (we have other clients like teleterm that may have to repeat the validation otherwise)

while we are on topic of validation, we also need to do the following:

• cannot leave subjects field empty
• cannot leave conditions field empty
• if subjects field contains access_request, then notifications field should not be empty
  • at least one recipient should be defined?
  • notification name must be defined

i propose to merge this PR as is, and create a separate PR to address backend validation?

also, i noticed the id field is always 0? should this be set to some value?

[EdwardDowling]

i propose to merge this PR as is, and create a separate PR to address backend validation?

also, i noticed the id field is always 0? should this be set to some value?

That sounds good to me.
The id field was deprecated and replaced with revision at some point i think

[rosstimothy]

@EdwardDowling The problem lies in the following lines in the storage layer.

https://github.com/gravitational/teleport/blob/master/lib/services/local/access_monitoring_rules.go#L77
https://github.com/gravitational/teleport/blob/master/lib/services/local/access_monitoring_rules.go#L87
https://github.com/gravitational/teleport/blob/master/lib/services/local/access_monitoring_rules.go#L97

The access monitoring rule object does not implement CheckAndSetDefaults which results in that line being a noop. Let's follow the mechanism that the notifications resource use to set server side generated and static fields and validate user provided fields: https://github.com/gravitational/teleport/blob/master/lib/services/local/notifications.go#L127-L155.

[public-teleport-github-review-bot]

@EdwardDowling See the table below for backport results.

Branch	Result
branch/v15	Failed