Draft: Initial suggestion - REST zone changelog endpoint(s)
REST way of how users may list the changes done in the zone(s)
Introduces two new REST endpoints:
/zones/changelog /zones/changelog/zone (filtered)
To fetch all zone changes from the audit log.
Merge request reports
Activity
This is probably not up to your code standards, but we needed this feature for our users to be able to check "who did what?" amongst all zones and zone data.
But I put it out here as-is, and then we can see if it's applicable for this app, and if bigger changes should be done (change log format of the audit log etc.)
(We also have an "inverse" of the audit log pattern matching, for all audit actions, eg. to see user creation/deletion and login/failed logins, but that's more OT than displaying the change log of the DNS data).
I have no big problem with that.
So far as I can see, there are some rights missing in my opinion (probably not good idea to send a changelogs to all users).
Could you add some short documentation (in the README file), and maybe some really simple test?
I have yet to try it.
Edited by Jan Hák
added 1 commit
- 3ae73bef - fix minimum auth for changelog + simple changelog tests
42 assert response.status_code == 200 43 44 def test_get_zones_changelog(auth): 45 ## Test API no-token 46 47 # try to get all zones 48 response = auth.get('/zones/changelog') 49 assert response.status_code == 401 50 51 # try to get all zones 52 response = auth.get('/zones/changelog/test') 53 assert response.status_code == 401 54 55 ## Test API with auth 56 auth.login('test_admin') 57 95 # Get arguments 96 if not zone: 97 zone = get_argument('zone') 98 99 # Get data 100 try: 101 loglines=[] 102 full_path_audit_log = current_app.config['audit-log'] 103 dir_path, file_name = os.path.split(current_app.config['audit-log']) 104 # iterate each file in a directory 105 for file in sorted(Path(dir_path).iterdir(), key=os.path.getmtime): 106 cur_path = os.path.join(dir_path, file) 107 if os.path.isfile(cur_path) and file_name in cur_path: 108 with open(cur_path, 'r') as file: 109 for line in file.readlines(): 110 if re.match("^(?:(?!(logged in|failed to log in|Registered user|Removed user)).)*$", line) is not None: We want the starting line(s) containing:
[2024-10-12 04:13:34,581]: User 'pettai' deleted record in zone 'example.net.'to be included in the returned change log as well, so we know who to cherish or blame
Edited by pettaiOh, I see..
I'll have to think about it.. Nice would be join username right into JSON record (comment I added to the
returnline further)..But at first glance seems a bit difficult, maybe after some format changes in audit log..
Edited by Jan Hák
107 if os.path.isfile(cur_path) and file_name in cur_path: 108 with open(cur_path, 'r') as file: 109 for line in file.readlines(): 110 if re.match("^(?:(?!(logged in|failed to log in|Registered user|Removed user)).)*$", line) is not None: 111 if zone is None: 112 loglines += line 113 elif zone in line: 114 loglines += line 115 116 except FileNotFoundError as e: 117 abort(404, e.strerror) 118 119 changelog = "" 120 changelog = changelog.join(loglines) 121 122 return changelog Please suggest if you have ideas, but it's quite handy to get the "diff" format for readability. I've never seen that in JSONified output anywhere...
Edited by pettaiI have still not I'm still not sure about the final form... The problem is that the two types of lines have quite different formats... My first thought was just something like this:
[ {"user": "test_admin", "message":"[2024-10-15 14:03:28,250]: User 'test_admin' add record(s) to zone 'test'"}, {"user": "test_admin", "message":"-\ttest.\t3600\tSOA\tdns1.test. hostmaster.test. 1 21600 3600 604800 86400'"}, ... ]Of course any feature like username, date, etc. in a separate entry would be a great feature to help filtering. But it depends what would be possible with actual format and what wouldn't.
Btw. I'm inclined to change the audit log format to better achieve this.
You can skip:
{"user": "test_admin", "message":"[2024-10-15 14:03:28,250]: User 'test_admin' add record(s) to zone 'test'"},if the log format would include timestamp(s)
{"timestamp": 1728953470, "user": "test_admin", "message":"-\ttest.\t3600\tSOA\tdns1.test. hostmaster.test. 1 21600 3600 604800 86400'"},etc.
98 99 # Get data 100 try: 101 loglines=[] 102 full_path_audit_log = current_app.config['audit-log'] 103 dir_path, file_name = os.path.split(current_app.config['audit-log']) 104 # iterate each file in a directory 105 for file in sorted(Path(dir_path).iterdir(), key=os.path.getmtime): 106 cur_path = os.path.join(dir_path, file) 107 if os.path.isfile(cur_path) and file_name in cur_path: 108 with open(cur_path, 'r') as file: 109 for line in file.readlines(): 110 if re.match("^(?:(?!(logged in|failed to log in|Registered user|Removed user)).)*$", line) is not None: 111 if zone is None: 112 loglines += line 113 elif zone in line: It seems to work only for "text" messages (messages like user xyz added/removed a record), but not for zone record changes (line starting with +-).
In my opinion, there is no way to decide if the change happened in the specified zone or in another... Because the record doesn't need to contain the zone name at all.
yeah, I know, I was talking about
elif zone in line:line.. This will not pass "record change lines" (lines starting with +-) when zone is specified, but zone name is not in the record name string..Change of format seems inevitable right now..
Edited by Jan Hákah, I see. The reason I wrote it like that was (to my knowledge), knot zone records are always written as FQDN. AFAICT, all our zones have all RRs in FQDN naming standard (I think it's a nice feature using absolute names rather than relative names). I realize this might not be true for all installations of knot or something you can change as a setting in knot?
I could be wrong, I wasn't aware we always return FQDN, I'll have to look..
But still, if I take it from the opposite side (the record name will contain the foreign zone name), it will pass, even if it is not a change to the specified zone.
Edited by Jan Hák
