-
Notifications
You must be signed in to change notification settings - Fork 357
fix: don't user hardcodec user in adk #802
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -146,15 +146,15 @@ func (a *kagentReconciler) reconcileAgentStatus(ctx context.Context, agent *v1al | |||||||
| conditionChanged := meta.SetStatusCondition(&agent.Status.Conditions, metav1.Condition{ | ||||||||
| Type: v1alpha2.AgentConditionTypeAccepted, | ||||||||
| Status: status, | ||||||||
| LastTransitionTime: metav1.Now(), | ||||||||
| Reason: reason, | ||||||||
| Message: message, | ||||||||
| ObservedGeneration: agent.Generation, | ||||||||
| }) | ||||||||
|
|
||||||||
| deployedCondition := metav1.Condition{ | ||||||||
| Type: v1alpha2.AgentConditionTypeReady, | ||||||||
| Status: metav1.ConditionUnknown, | ||||||||
| LastTransitionTime: metav1.Now(), | ||||||||
| ObservedGeneration: agent.Generation, | ||||||||
|
||||||||
| ObservedGeneration: agent.Generation, | |
| ObservedGeneration: agent.Generation, | |
| LastTransitionTime: metav1.Now(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
controller runtime handles it automatically. it is not needed.
Copilot
AI
Aug 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The LastTransitionTime field is removed from this condition as well. This field should be included to properly track when the condition state changed.
| ObservedGeneration: toolServer.Generation, | |
| ObservedGeneration: toolServer.Generation, | |
| LastTransitionTime: metav1.Now(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
controller runtime handles it automatically. it is not needed.
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -27,9 +27,6 @@ | |||||||||||||
| from ._task_store import KAgentTaskStore | ||||||||||||||
| from ._token import KAgentTokenService | ||||||||||||||
|
|
||||||||||||||
| # --- Constants --- | ||||||||||||||
| USER_ID = "admin@kagent.dev" | ||||||||||||||
|
|
||||||||||||||
| # --- Configure Logging --- | ||||||||||||||
| logger = logging.getLogger(__name__) | ||||||||||||||
|
|
||||||||||||||
|
|
@@ -40,7 +37,7 @@ def __init__(self, user_id: str): | |||||||||||||
|
|
||||||||||||||
| @property | ||||||||||||||
| def is_authenticated(self) -> bool: | ||||||||||||||
| return False | ||||||||||||||
| return True | ||||||||||||||
|
|
||||||||||||||
| @property | ||||||||||||||
| def user_name(self) -> str: | ||||||||||||||
|
|
@@ -52,9 +49,8 @@ class KAgentRequestContextBuilder(SimpleRequestContextBuilder): | |||||||||||||
| A request context builder that will be used to hack in the user_id for now. | ||||||||||||||
| """ | ||||||||||||||
|
|
||||||||||||||
| def __init__(self, user_id: str, task_store: TaskStore): | ||||||||||||||
| def __init__(self, task_store: TaskStore): | ||||||||||||||
| super().__init__(task_store=task_store) | ||||||||||||||
| self.user_id = user_id | ||||||||||||||
|
|
||||||||||||||
| async def build( | ||||||||||||||
| self, | ||||||||||||||
|
|
@@ -64,10 +60,12 @@ async def build( | |||||||||||||
| task: Task | None = None, | ||||||||||||||
| context: ServerCallContext | None = None, | ||||||||||||||
| ) -> RequestContext: | ||||||||||||||
| if not context: | ||||||||||||||
| context = ServerCallContext(user=KAgentUser(user_id=self.user_id)) | ||||||||||||||
| else: | ||||||||||||||
| context.user = KAgentUser(user_id=self.user_id) | ||||||||||||||
| if context: | ||||||||||||||
| # grab the user id from the header | ||||||||||||||
| headers = context.state.get("headers", {}) | ||||||||||||||
| user_id = headers.get("x-user-id", None) | ||||||||||||||
| if user_id: | ||||||||||||||
| context.user = KAgentUser(user_id=user_id) | ||||||||||||||
|
||||||||||||||
| context.user = KAgentUser(user_id=user_id) | |
| if user_id and is_valid_user_id(user_id): | |
| context.user = KAgentUser(user_id=user_id) | |
| else: | |
| if user_id is not None: | |
| logger.warning(f"Invalid user_id in x-user-id header: {user_id!r}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's true, but not worse than current state. the auth story will evolve over time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The LastTransitionTime field is removed but should be set when creating status conditions. Without this field, the condition may not properly track when state changes occurred, which is important for Kubernetes status reporting.