Skip to content

Conversation

@areed
Copy link
Contributor

@areed areed commented Nov 6, 2025

No description provided.

Copy link

@maraino maraino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't look into all of this, but added a few comments.

Comment on lines 108 to 111
"value_from_extension": schema.StringAttribute{
Computed: true,
MarkdownDescription: replyAttrsProps["valueFromExtension"],
},
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the moment this is not "really" supported. Right now, the OIDs supported are only the ones in the subject, for example, common name is 2.5.4.3, and the subject is not really an extension. At some point we might implement this for extensions but only for simple extensions where we can get an string from the extension value.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"value_from_extension": types.StringType,
}

func toAPI(ctx context.Context, diags *diag.Diagnostics, model *ManagedRadiusModel) v20250101.ManagedRadius {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although the providers uses the toAPI and fromAPI methods, when I added the strategies I had to generalize some models, and add a FromAPI and a Model.ToAPI methods. Something like this

type Model struct{
  ID types.String ``tfsdk:"id"`
  // ...
}

func FromAPI(ctx context.Context, v *v20250101.XXX, state utils.AttributeGetter, root path.Path) (types.Object, diag.Diagnostics) {
  // ...
}

func (m *Model) ToAPI(ctx context.Context, obj types.Object) (*v20250101.XXX, diag.Diagnostics) {
  // ...
}

It might not be necessary right now, but if we need to compose multiple models it really helps.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines +124 to +127
"server_port": schema.StringAttribute{
MarkdownDescription: props["serverPort"],
Computed: true,
},
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we used this for RadSec there are actually two ports, well, 3 taking into account accounting:

  • 1812 radius
  • 1813 radius accounting
  • 2083 RadSec (when enabled)

At this moment regular radius is always enabled even if RadSec is enabled, but this might change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RadSec will have to be supported in the API before it can be added here.

@areed areed requested a review from dopey December 1, 2025 17:46
@areed areed merged commit dcf8930 into main Dec 1, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants