#5524 closed enhancement (complete)

Create stub Radius library

Reported by: tomek Owned by: fdupont
Priority: medium Milestone: Kea1.4
Component: hook-radius Version: git
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DHCP Feature Depending on Ticket:
Estimated Difficulty: 0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description (last modified by fdupont)

The RadiusDesign call for creation of a Radius hook library.

This ticket covers:

  • creation of a hook library
  • extension to configure.ac to detect FreeRadius-client presence
  • modification of makefiles (so the Radius library is only built when FreeRadius is detected)

Subtickets

Attachments (1)

freeradius-client.rb (401 bytes) - added by fdupont 21 months ago.
Brew file for FreeRADIUS client

Download all attachments as: .zip

Change History (9)

comment:1 Changed 21 months ago by fdupont

  • Owner set to fdupont
  • Status changed from new to accepted

comment:2 Changed 21 months ago by fdupont

  • Description modified (diff)

Note the library is not FreeRADIUS server / FreeRADIUS but FreeRADIUS client, cf http://wiki.freeradius.org/project/Radiusclient

Changed 21 months ago by fdupont

Brew file for FreeRADIUS client

comment:3 Changed 21 months ago by fdupont

  • Owner changed from fdupont to UnAssigned
  • Status changed from accepted to reviewing

Done. The DSO only initializes a FreeRADIUS client library handle. In particular it does not config something (or is configured)...

comment:4 Changed 20 months ago by tomek

  • Owner changed from UnAssigned to tomek

comment:5 follow-up: Changed 20 months ago by tomek

  • Owner changed from tomek to fdupont

The code didn't compile for me on CentOS 7. I had to make a small
tweak in the Makefile.am in src/hooks/dhcp/radius_client. Please pull
and review. In essence, the path was passed to compiler, but without
-I. If you have FreeRadius client in the default path, you'd never
notice it not working.

The configure script should print out something when
--with-freeradius option is used. Actually, it should
print out something every time (either FreeRadius client: No
or FreeRadius client: datails here).

The library should be called simply radius, not radius client.
Adding client would suggest that we will one day also implement
a server, which we will never do. I think the libraries should have as
simple names as possible.

Consequently, the namespace used should be isc::radius, not
isc::radius_client.

radius_client.h

The class should have more useful name. It is clear that it is radius
related (it is in isc::radius namespace). I don't have any specific
preference, but RC would be very cryptic for Thomas or Marcin.
Maybe calling it RadiusImpl be the simplest choice?

load_unload_unittests.cc
This is I think the sixth copy of LibLoadTest class :)
One day we will move it to testutils and remove all other instances.


This change definitely requires a changelog entry. How about this:

24.	[func]		fdupont
	Initial skeleton implementation for Radius hook library added.
	(Trac #5524, git tbd)

comment:6 in reply to: ↑ 5 Changed 20 months ago by fdupont

Replying to tomek:

The code didn't compile for me on CentOS 7. I had to make a small
tweak in the Makefile.am in src/hooks/dhcp/radius_client. Please pull
and review. In essence, the path was passed to compiler, but without
-I. If you have FreeRadius client in the default path, you'd never
notice it not working.

=> I am afraid I did not resume my CentOS or RHEL 7 VM to check...

The configure script should print out something when
--with-freeradius option is used. Actually, it should
print out something every time (either FreeRadius client: No
or FreeRadius client: datails here).

=> matter of taste and BTW trivial to fix.

The library should be called simply radius, not radius client.
Adding client would suggest that we will one day also implement
a server, which we will never do. I think the libraries should have as
simple names as possible.

=> this is the right time to agree about the best name. I am removing
client from everywhere (easier now than laster).

Consequently, the namespace used should be isc::radius, not
isc::radius_client.

=> same

The class should have more useful name. It is clear that it is radius
related (it is in isc::radius namespace). I don't have any specific
preference, but RC would be very cryptic for Thomas or Marcin.
Maybe calling it RadiusImpl be the simplest choice?

=> RadiusClient? was a bit long. now Radius is free so I am change RC into Radius.

load_unload_unittests.cc
This is I think the sixth copy of LibLoadTest class :)
One day we will move it to testutils and remove all other instances.

=> perhaps a header could do the job. BTW I am afraid there is another
instance in the host cache tests...

This change definitely requires a changelog entry. How about this:

24.	[func]		fdupont
	Initial skeleton implementation for Radius hook library added.
	(Trac #5524, git tbd)

=> does this mean I do the pull, change, check if it still works (i.e. no typo)
and merge?

Working on the branch.

comment:7 Changed 20 months ago by fdupont

Moved the -I to configure. Removed all client in names / comments, renamed RC. I have a make checking it is good. If tests pass I'll merge.

comment:8 Changed 20 months ago by fdupont

  • Resolution set to complete
  • Status changed from reviewing to closed

Fixed configure messages (in fact in premium/config.m4). Merged. Closing.

Note: See TracTickets for help on using tickets.