Skip to content

Feature: Add Global Admin-Defined EC2 Limits for Autograders#2320

Open
Giabbi wants to merge 3 commits intoec2-testingfrom
giancarlo/admin-ec2-limits
Open

Feature: Add Global Admin-Defined EC2 Limits for Autograders#2320
Giabbi wants to merge 3 commits intoec2-testingfrom
giancarlo/admin-ec2-limits

Conversation

@Giabbi
Copy link
Copy Markdown
Member

@Giabbi Giabbi commented Apr 15, 2026

Description

This PR introduces the ability for global administrators to restrict which AWS EC2 instance types are available to instructors when configuring an assessment's autograder.

Key changes:

  • Global Admin UI: Added a new "EC2 Limits" tab to the admins/autolab_config page.
  • Backend Storage: Created Ec2ConfigController to save selected instances to a global config/ec2_config.yml file, matching Autolab's existing configuration pattern for LTI and SMTP.
  • Instructor UI Refactor: Cleaned up the hardcoded AWS logic in app/views/autograders/edit.html.erb. The dropdown is now dynamically populated.
  • Controller Hook: Updated AutogradersController#edit to read from ec2_config.yml and filter the instructor's dropdown. If the config file does not exist, it safely falls back to the full Autograder::INSTANCE_TYPES list.

(Note: Per discussion with Soma, moving the EC2 hardware descriptions themselves into a dynamic YAML dictionary is deferred to next semester. This PR focuses strictly on the global filtering mechanism).

Motivation and Context

Previously, instructors were exposed to a hardcoded list of all possible EC2 instances supported by Autolab. This posed a risk of over-provisioning expensive compute resources (eg, an instructor accidentally selecting a GPU instance for a basic "Hello World" C assignment). This change gives Computing Services (or whoever the admin is in the org) centralized control over AWS resource allocation while keeping the instructor experience streamlined.

How Has This Been Tested?

Tested locally on development environment:

  1. Logged in as a global administrator, navigated to "Configure Autolab", and verified the "EC2 Limits" tab renders correctly.
  2. Selected a subset of instances (e.g., t2.micro, t3.medium) and verified they successfully save to config/ec2_config.yml.
  3. Logged in as an instructor, navigated to an assessment's "Edit Autograder" page, and verified the dropdown only displays the instances checked by the admin.
  4. Verified that the Autograder model validation remains intact (manually sending an invalid instance string via POST request is successfully rejected).
  5. Tested the fallback behavior (verifying the UI doesn't crash if ec2_config.yml hasn't been generated yet).

Types of changes

[ ] Bug fix (non-breaking change which fixes an issue)

[x] New feature (non-breaking change which adds functionality)

[ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

[x] I have run rubocop and erblint for style check. If you haven't, run overcommit --install && overcommit --sign to use pre-commit hook for linting

[ ] My change requires a change to the documentation, which is located at Autolab Docs

[ ] I have updated the documentation accordingly, included in this PR

Other issues / help required

Documentation: I left the documentation checkboxes blank, but this feature likely requires a small update to the Autolab Docs to explain the new "EC2 Limits" tab under "Configure Autolab". I can write that up if needed!

Giabbi added 2 commits April 12, 2026 14:08
… instance selection. TODO: connect array to admin panel so that admin can select which instance to allow
…lled back previous database schema change in favor of a global approach with a yaml config file
@Giabbi Giabbi requested review from KesterTan and melissajinn April 15, 2026 01:57
@Giabbi Giabbi changed the base branch from master to ec2-testing April 15, 2026 02:01
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 15, 2026

Caution

Review failed

Failed to post review comments

📝 Walkthrough

Walkthrough

This pull request introduces EC2 SSH support for autograders with encrypted credential management, instance type selection, and administrative configuration. It includes database migrations for encrypted storage, controller and view refactoring with tabbed UI layouts, updated job parameter handling for EC2 integration, and enhanced authorization and validation rules across multiple layers.

Changes

Cohort / File(s) Summary
EC2 Configuration & Admin
app/controllers/ec2_config_controller.rb, app/views/admins/_ec2_config.html.erb, app/views/admins/autolab_config.html.erb
New EC2 config controller and admin UI for managing allowed instance types; loads/persists configuration to YAML file; extends admin config page with EC2 limits tab.
Autograder Model
app/models/autograder.rb
Added encryption for access_key and access_key_id attributes; introduced INSTANCE_TYPES constant; added validations for instance_type (allowlist), autograde_timeout (inclusion range), and conditional access-key validations when use_access_key? is enabled.
Autograder Controller
app/controllers/autograders_controller.rb
Modified create, edit, and update actions to handle EC2 settings (instance type, access keys); added conditional ec2_config.yml reading; normalized use_access_key to boolean; conditionally clears or preserves access keys; expanded strong parameters.
Autograder Views
app/views/autograders/_form.html.erb, app/views/autograders/_basic_settings.html.erb, app/views/autograders/_ec2_settings.html.erb, app/assets/javascripts/autograder.js
Refactored form into tabbed layout (basic/EC2); added partials for settings organization; introduced JavaScript for checkbox-driven field enable/disable and tooltip/hover behavior on instance type options.
Autograder Admin & Course Models
app/models/course.rb, app/controllers/admins_controller.rb
Added allowed_ec2_instances serialized array attribute to Course; extended AdminsController#autolab_config to load and set @ec2_config_hash from ec2_config.yml or default configuration.
Job & Score Processing
app/helpers/assessment_autograde_core.rb, app/controllers/jobs_controller.rb, app/controllers/scoreboards_controller.rb
Updated tango_add_job to conditionally append EC2 parameters (ec2Vmms, instanceType, accessKey, accessKeyId) when EC2 SSH is enabled; refined job state derivation to check for "Autodriver returned normally" and "Error" markers; enhanced scoreboard JSON parsing with array-length padding and validation.
Authorization & Parameters
app/controllers/assessments_controller.rb, app/controllers/announcements_controller.rb
Added bulkExport, bulkGrade_complete, and regradeBatch authorization rules; hardened assessment parameters by explicitly deleting restricted fields; made announcement_params dynamic to conditionally permit :system only for administrators.
Removed Components
app/controllers/scores_controller.rb, spec/controllers/scores_controller_spec.rb, config/routes.rb
Removed entire ScoresController (previously handled per-submission score CRUD); removed corresponding routes and empty test spec.
Database Migrations & Schema
db/migrate/20241205233214_add_ec2_ssh_fields_to_autograders.rb, db/migrate/20241211042124_add_use_access_key_to_autograder.rb, db/migrate/20251021034813_encrypt_autograder_credentials.rb, db/schema.rb
Three migrations: added instance_type, access_key_ciphertext, access_key_id_ciphertext, and use_access_key columns to autograders; encrypted existing plaintext credentials; updated schema version and adjusted column types (datetime → timestamp, integer → bigint where applicable); added ssh_keys table and explicit charset/collation.
Configuration & Feature Flags
config/environments/development.rb, config/environments/production.rb.template, config/initializers/filter_parameter_logging.rb, config/routes.rb
Added config.x.ec2_ssh feature flag (enabled in development, disabled in production); extended filtered parameters to include :access_key and :access_key_id; added route for EC2 config update action.
Tests & Factories
spec/factories/autograders.rb, spec/models/autograder_spec.rb, spec/helpers/assessment_autograde_core_spec.rb, app/views/scoreboards/show.html.erb
Updated autograder factory to set instance_type and use_access_key defaults; added comprehensive validation tests for instance type and access-key scenarios; added helper spec for EC2 parameter inclusion logic; hardened scoreboard view with nil-safety checks on array indexing.

Sequence Diagram(s)

sequenceDiagram
    participant Admin as Administrator
    participant Web as Autograder Form
    participant Ctrl as Autograders<br/>Controller
    participant Model as Autograder<br/>Model
    participant DB as Database<br/>(Encrypted)
    participant TangoHelper as tango_add_job<br/>Helper
    participant Tango as TangoClient<br/>Job Queue

    Admin->>Web: Fill EC2 settings form<br/>(instance type, access keys)
    Web->>Ctrl: POST update with params
    Ctrl->>Ctrl: Normalize use_access_key<br/>to boolean
    Ctrl->>Ctrl: Conditionally clear or<br/>preserve access keys
    Ctrl->>Model: Save autograder with<br/>EC2 params
    Model->>Model: Validate instance_type<br/>against INSTANCE_TYPES
    Model->>Model: Validate access_key_id<br/>format if use_access_key?
    Model->>DB: Encrypt & persist<br/>access_key credentials
    DB-->>Model: Confirmation
    Model-->>Ctrl: Save success

    Note over Tango: Later, on autograde trigger:
    Ctrl->>TangoHelper: Call tango_add_job
    TangoHelper->>TangoHelper: Check Rails.config.x.ec2_ssh
    alt EC2 SSH Enabled
        TangoHelper->>TangoHelper: Add ec2Vmms: true
        TangoHelper->>TangoHelper: Add instanceType
        alt use_access_key?
            TangoHelper->>TangoHelper: Add accessKey,<br/>accessKeyId
        end
    end
    TangoHelper->>Tango: POST job with<br/>conditional EC2 params
    Tango-->>TangoHelper: Job ID
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • Autograder improvements #2209: Modifies AutogradersController#edit and related autograder form handling logic, overlapping with EC2 settings and instance type management in this PR.

Suggested reviewers

  • dwang3851
  • coder6583
  • evanyeyeye
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 13.64% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The PR title clearly summarizes the main feature: adding global admin-defined EC2 limits for autograders, which aligns with the core objective and changes throughout the changeset.
Description check ✅ Passed The PR description comprehensively covers all required sections: clear description of changes, motivation/context for resource control, detailed testing methodology, correct feature checkbox selected, and acknowledgment of documentation needs.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch giancarlo/admin-ec2-limits

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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.

3 participants